| Message ID | 20190809124553.67012-2-mika.westerberg@linux.intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | watchdog: Correct iTCO for Cannon Lake and beyond | expand |
On Fri, Aug 09, 2019 at 03:45:52PM +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@linux.intel.com> Acked-by: Guenter Roeck <linux@roeck-us.net> I assume this will be applied through i2c together with the other patch of the series. Guenter > --- > 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); > + outw(val, TCO1_CNT(p)); > + newval = inw(TCO1_CNT(p)); > + > + /* make sure the update is successful */ > + return val != newval ? -EIO : 0; > +} > + > static void iTCO_wdt_no_reboot_bit_setup(struct iTCO_wdt_private *p, > struct itco_wdt_platform_data *pdata) > { > @@ -224,7 +241,9 @@ static void iTCO_wdt_no_reboot_bit_setup(struct iTCO_wdt_private *p, > return; > } > > - if (p->iTCO_version >= 2) > + if (p->iTCO_version >= 6) > + p->update_no_reboot_bit = update_no_reboot_bit_cnt; > + else if (p->iTCO_version >= 2) > p->update_no_reboot_bit = update_no_reboot_bit_mem; > else if (p->iTCO_version == 1) > p->update_no_reboot_bit = update_no_reboot_bit_pci; > @@ -452,7 +471,8 @@ static int iTCO_wdt_probe(struct platform_device *pdev) > * Get the Memory-Mapped GCS or PMC register, we need it for the > * NO_REBOOT flag (TCO v2 and v3). > */ > - if (p->iTCO_version >= 2 && !pdata->update_no_reboot_bit) { > + if (p->iTCO_version >= 2 && p->iTCO_version < 6 && > + !pdata->update_no_reboot_bit) { > p->gcs_pmc_res = platform_get_resource(pdev, > IORESOURCE_MEM, > ICH_RES_MEM_GCS_PMC); > @@ -502,6 +522,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) > > /* Clear out the (probably old) status */ > switch (p->iTCO_version) { > + case 6: > case 5: > case 4: > outw(0x0008, TCO1_STS(p)); /* Clear the Time Out Status bit */ > -- > 2.20.1 >
Hi Mika, 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@linux.intel.com> > --- > 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()? > + 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@suse.de>
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@linux.intel.com> > > --- > > 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@suse.de> Thanks!
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); + outw(val, TCO1_CNT(p)); + newval = inw(TCO1_CNT(p)); + + /* make sure the update is successful */ + return val != newval ? -EIO : 0; +} + static void iTCO_wdt_no_reboot_bit_setup(struct iTCO_wdt_private *p, struct itco_wdt_platform_data *pdata) { @@ -224,7 +241,9 @@ static void iTCO_wdt_no_reboot_bit_setup(struct iTCO_wdt_private *p, return; } - if (p->iTCO_version >= 2) + if (p->iTCO_version >= 6) + p->update_no_reboot_bit = update_no_reboot_bit_cnt; + else if (p->iTCO_version >= 2) p->update_no_reboot_bit = update_no_reboot_bit_mem; else if (p->iTCO_version == 1) p->update_no_reboot_bit = update_no_reboot_bit_pci; @@ -452,7 +471,8 @@ static int iTCO_wdt_probe(struct platform_device *pdev) * Get the Memory-Mapped GCS or PMC register, we need it for the * NO_REBOOT flag (TCO v2 and v3). */ - if (p->iTCO_version >= 2 && !pdata->update_no_reboot_bit) { + if (p->iTCO_version >= 2 && p->iTCO_version < 6 && + !pdata->update_no_reboot_bit) { p->gcs_pmc_res = platform_get_resource(pdev, IORESOURCE_MEM, ICH_RES_MEM_GCS_PMC); @@ -502,6 +522,7 @@ static int iTCO_wdt_probe(struct platform_device *pdev) /* Clear out the (probably old) status */ switch (p->iTCO_version) { + case 6: case 5: case 4: outw(0x0008, TCO1_STS(p)); /* Clear the Time Out Status bit */
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@linux.intel.com> --- drivers/watchdog/iTCO_wdt.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)