diff mbox series

[1/2] watchdog: iTCO: Add support for Cannon Lake PCH iTCO

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

Commit Message

Mika Westerberg Aug. 9, 2019, 12:45 p.m. UTC
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(-)

Comments

Guenter Roeck Aug. 9, 2019, 8:49 p.m. UTC | #1
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
>
Jean Delvare Aug. 28, 2019, 12:50 p.m. UTC | #2
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>
Mika Westerberg Aug. 28, 2019, 1:04 p.m. UTC | #3
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 mbox series

Patch

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