2019-03-27 06:45:00

by Ji-Ze Hong (Peter Hong)

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

Separate declaration and assignment in watchdog_start()

Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
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

2019-03-27 06:44:00

by Ji-Ze Hong (Peter Hong)

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

Fix error bit operation in watchdog_start()

Fixes: 14b24a88a3660 ("watchdog: f71808e_wdt: Add F81866 support")
Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
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

2019-03-27 17:33:14

by Guenter Roeck

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

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) <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

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
>

2019-03-27 17:34:50

by Guenter Roeck

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

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) <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

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