On Wed, Aug 28, 2019 at 02:50:34PM +0200, Jean Delvare wrote: > Hi Mika, Hi, > On Fri, 9 Aug 2019 15:45:52 +0300, Mika Westerberg wrote: > > In Intel Cannon Lake PCH the NO_REBOOT bit was moved from the private > > register space to be part of the TCO1_CNT register. For this reason > > introduce another version (6) that uses this register to set and clear > > NO_REBOOT bit. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > > --- > > drivers/watchdog/iTCO_wdt.c | 25 +++++++++++++++++++++++-- > > 1 file changed, 23 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c > > index c559f706ae7e..505f2c837347 100644 > > --- a/drivers/watchdog/iTCO_wdt.c > > +++ b/drivers/watchdog/iTCO_wdt.c > > @@ -215,6 +215,23 @@ static int update_no_reboot_bit_mem(void *priv, bool set) > > return 0; > > } > > > > +static int update_no_reboot_bit_cnt(void *priv, bool set) > > +{ > > + struct iTCO_wdt_private *p = priv; > > + u16 val, newval; > > + > > + val = inw(TCO1_CNT(p)); > > + if (set) > > + val |= BIT(0); > > + else > > + val &= ~BIT(0); > > Are you not supposed to include <linux/bits.h> before you use BIT()? Apparently not because it compiles just fine without ;-) Probably gets includes via another header. I'll add it in v2. > > + outw(val, TCO1_CNT(p)); > > + newval = inw(TCO1_CNT(p)); > > + > > + /* make sure the update is successful */ > > + return val != newval ? -EIO : 0; > > +} > > Other than this minor nitpick, looks good to me. > > Reviewed-by: Jean Delvare <jdelvare@xxxxxxx> Thanks!