[PATCH 00/10] Intel Keem Bay WDT bug fixes

[PATCH 00/10] Intel Keem Bay WDT bug fixes

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

The series of patches include the below bug fixes
in the Intel Keem Bay watchdog timer driver:

Patch 1/10:
- Update WDT pre-timeout during the initialization
  The pretimeout register has a default reset value. Hence
  when a smaller WDT timeout is set which would be lesser than the
  default pretimeout, the system behaves abnormally, starts
  triggering the pretimeout interrupt even when the WDT is
  not enabled, most of the times leading to system crash.
  Hence an update in the pre-timeout is also required for the
  default timeout that is being configured.

Patch 2/10:
- Upadate WDT pretimeout for every update in timeout
  The pre-timeout value to be programmed to the register has to be
  calculated and updated for every change in the timeout value.
  Else the threshold time wouldn't be calculated to its
  corresponding timeout.

Patch 3/10:
- Update pretimeout to 0 in the TH ISR
  The pretimeout has to be updated to 0 during the ISR of the
  ThresHold interrupt. Else the TH interrupt would be triggerred for
  every tick until the timeout.

Patch 4/10:
- Clear either the TO or TH interrupt bit
  During the interrupt service routine of the TimeOut interrupt and
  the ThresHold interrupt, the respective interrupt clear bit
  have to be cleared and not both.

Patch 5/10:
- Remove timeout update in the WDT start function
  Removed set timeout from the start WDT function. There is a function
  defined to set the timeout. Hence no need to set the timeout again in
  start function as the timeout would have been already updated
  before calling the start/enable.

Patch 6/10:
- Removed timeout update in the TO ISR
  In the TO ISR removed updating the Timeout value because
  its not serving any purpose as the timer would have already expired
  and the system would be rebooting.

Patch 7/10:
- Update the check in keembay_wdt_resume()
  Corrected the typo in the function keembay_wdt_resume, we need to
  enable the WDT if it is disabled/not active.

Patch 8/10:
- MACRO for WDT enable and disable values
  Introduced MACRO's for WDT enable and disable values for better readability

Patch 9/10:
- WDT SMC handler MACRO name update
  Updated the WDT SMC handler MACRO name to make it clear that its
  a ARM SMC handler that helps in clearing the WDT interrupt bit.

Patch 10/10:
- Typo corrections and other blank operations
  Corrected typos, aligned the tabs and added new lines
  wherever required for better readability

Shruthi Sanil (10):
  watchdog: keembay: Update WDT pre-timeout during the initialization
  watchdog: keembay: Upadate WDT pretimeout for every update in timeout
  watchdog: keembay: Update pretimeout to zero in the TH ISR
  watchdog: keembay: Clear either the TO or TH interrupt bit
  watchdog: keembay: Remove timeout update in the WDT start function
  watchdog: keembay: Removed timeout update in the TO ISR
  watchdog: keembay: Update the check in keembay_wdt_resume()
  watchdog: keembay: MACRO for WDT enable and disable values
  watchdog: keembay: WDT SMC handler MACRO name update
  watchdog: keembay: Typo corrections and other blank operations

 drivers/watchdog/keembay_wdt.c | 36 ++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 13 deletions(-)


base-commit: 88b06399c9c766c283e070b022b5ceafa4f63f19
-- 
2.17.1

[PATCH 01/10] watchdog: keembay: Update WDT pre-timeout during the initialization

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

The pretimeout register has a default reset value. Hence
when a smaller WDT timeout is set which would be lesser than the
default pretimeout, the system behaves abnormally, starts
triggering the pretimeout interrupt even when the WDT is
not enabled, most of the times leading to system crash.
Hence an update in the pre-timeout is also required for the
default timeout that is being configured.

Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 547d3fea33ff..f2f5c9fae29c 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -29,6 +29,7 @@
 #define WDT_LOAD_MAX		U32_MAX
 #define WDT_LOAD_MIN		1
 #define WDT_TIMEOUT		5
+#define WDT_PRETIMEOUT		4
 
 static unsigned int timeout = WDT_TIMEOUT;
 module_param(timeout, int, 0);
@@ -224,11 +225,13 @@ static int keembay_wdt_probe(struct platform_device *pdev)
 	wdt->wdd.min_timeout	= WDT_LOAD_MIN;
 	wdt->wdd.max_timeout	= WDT_LOAD_MAX / wdt->rate;
 	wdt->wdd.timeout	= WDT_TIMEOUT;
+	wdt->wdd.pretimeout	= WDT_PRETIMEOUT;
 
 	watchdog_set_drvdata(&wdt->wdd, wdt);
 	watchdog_set_nowayout(&wdt->wdd, nowayout);
 	watchdog_init_timeout(&wdt->wdd, timeout, dev);
 	keembay_wdt_set_timeout(&wdt->wdd, wdt->wdd.timeout);
+	keembay_wdt_set_pretimeout(&wdt->wdd, wdt->wdd.pretimeout);
 
 	ret = devm_watchdog_register_device(dev, &wdt->wdd);
 	if (ret)
-- 
2.17.1

[PATCH 02/10] watchdog: keembay: Upadate WDT pretimeout for every update in timeout

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

The pre-timeout value to be programmed to the register has to be
calculated and updated for every change in the timeout value.
Else the threshold time wouldn't be calculated to its
corresponding timeout.

Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index f2f5c9fae29c..b2afeb4a60e3 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -109,6 +109,7 @@ static int keembay_wdt_set_timeout(struct watchdog_device *wdog, u32 t)
 {
 	wdog->timeout = t;
 	keembay_wdt_set_timeout_reg(wdog);
+	keembay_wdt_set_pretimeout_reg(wdog);
 
 	return 0;
 }
-- 
2.17.1

[PATCH 03/10] watchdog: keembay: Update pretimeout to zero in the TH ISR

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

The pretimeout has to be updated to zero during the ISR of the
ThresHold interrupt. Else the TH interrupt would be triggerred for
every tick until the timeout.

Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index b2afeb4a60e3..6053416b8d3d 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -154,6 +154,8 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
 	struct keembay_wdt *wdt = dev_id;
 	struct arm_smccc_res res;
 
+	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
+
 	arm_smccc_smc(WDT_ISR_CLEAR, WDT_ISR_MASK, 0, 0, 0, 0, 0, 0, &res);
 	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
 	watchdog_notify_pretimeout(&wdt->wdd);
-- 
2.17.1

[PATCH 04/10] watchdog: keembay: Clear either the TO or TH interrupt bit

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

During the interrupt service routine of the TimeOut interrupt and
the ThresHold interrupt, the respective interrupt clear bit
have to be cleared and not both.

Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 6053416b8d3d..f2a16c9933c3 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -23,7 +23,8 @@
 #define TIM_WDOG_EN		0x8
 #define TIM_SAFE		0xc
 
-#define WDT_ISR_MASK		GENMASK(9, 8)
+#define WDT_TH_INT_MASK		BIT(8)
+#define WDT_TO_INT_MASK		BIT(9)
 #define WDT_ISR_CLEAR		0x8200ff18
 #define WDT_UNLOCK		0xf1d0dead
 #define WDT_LOAD_MAX		U32_MAX
@@ -142,7 +143,7 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
 	struct arm_smccc_res res;
 
 	keembay_wdt_writel(wdt, TIM_WATCHDOG, 1);
-	arm_smccc_smc(WDT_ISR_CLEAR, WDT_ISR_MASK, 0, 0, 0, 0, 0, 0, &res);
+	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
 	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
 	emergency_restart();
 
@@ -156,7 +157,7 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
 
 	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
 
-	arm_smccc_smc(WDT_ISR_CLEAR, WDT_ISR_MASK, 0, 0, 0, 0, 0, 0, &res);
+	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
 	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
 	watchdog_notify_pretimeout(&wdt->wdd);
 
-- 
2.17.1

[PATCH 05/10] watchdog: keembay: Remove timeout update in the WDT start function

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

Removed set timeout from the start WDT function. There is a function
defined to set the timeout. Hence no need to set the timeout again in
start function as the timeout would have been already updated
before calling the start/enable.

Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index f2a16c9933c3..039753b9932b 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -84,7 +84,6 @@ static int keembay_wdt_start(struct watchdog_device *wdog)
 {
 	struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
 
-	keembay_wdt_set_timeout_reg(wdog);
 	keembay_wdt_writel(wdt, TIM_WDOG_EN, 1);
 
 	return 0;
-- 
2.17.1

[PATCH 06/10] watchdog: keembay: Removed timeout update in the TO ISR

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

In the TO ISR removed updating the Timeout value because
its not serving any purpose as the timer would have already expired
and the system would be rebooting.

Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 039753b9932b..dd192b8dff55 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -141,7 +141,6 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
 	struct keembay_wdt *wdt = dev_id;
 	struct arm_smccc_res res;
 
-	keembay_wdt_writel(wdt, TIM_WATCHDOG, 1);
 	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
 	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
 	emergency_restart();
-- 
2.17.1

[PATCH 07/10] watchdog: keembay: Update the check in keembay_wdt_resume()

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

Corrected the typo in the function keembay_wdt_resume, we need to
enable the WDT if it is disabled/not active.

Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index dd192b8dff55..10896415f8c7 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -260,7 +260,7 @@ static int __maybe_unused keembay_wdt_resume(struct device *dev)
 {
 	struct keembay_wdt *wdt = dev_get_drvdata(dev);
 
-	if (watchdog_active(&wdt->wdd))
+	if (!watchdog_active(&wdt->wdd))
 		return keembay_wdt_start(&wdt->wdd);
 
 	return 0;
-- 
2.17.1

[PATCH 08/10] watchdog: keembay: MACRO for WDT enable and disable values

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

Introduced MACRO's for WDT enable and disable values for better readability

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 10896415f8c7..63a7c5d719a3 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -27,6 +27,8 @@
 #define WDT_TO_INT_MASK		BIT(9)
 #define WDT_ISR_CLEAR		0x8200ff18
 #define WDT_UNLOCK		0xf1d0dead
+#define WDT_DISABLE		0x0
+#define WDT_ENABLE		0x1
 #define WDT_LOAD_MAX		U32_MAX
 #define WDT_LOAD_MIN		1
 #define WDT_TIMEOUT		5
@@ -84,7 +86,7 @@ static int keembay_wdt_start(struct watchdog_device *wdog)
 {
 	struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
 
-	keembay_wdt_writel(wdt, TIM_WDOG_EN, 1);
+	keembay_wdt_writel(wdt, TIM_WDOG_EN, WDT_ENABLE);
 
 	return 0;
 }
@@ -93,7 +95,7 @@ static int keembay_wdt_stop(struct watchdog_device *wdog)
 {
 	struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
 
-	keembay_wdt_writel(wdt, TIM_WDOG_EN, 0);
+	keembay_wdt_writel(wdt, TIM_WDOG_EN, WDT_DISABLE);
 
 	return 0;
 }
-- 
2.17.1

[PATCH 09/10] watchdog: keembay: WDT SMC handler MACRO name update

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

Updated the WDT SMC handler MACRO name to make it clear that its
a ARM SMC handler that helps in clearing the WDT interrupt bit.

Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 63a7c5d719a3..0a8cf5b35699 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -25,7 +25,7 @@
 
 #define WDT_TH_INT_MASK		BIT(8)
 #define WDT_TO_INT_MASK		BIT(9)
-#define WDT_ISR_CLEAR		0x8200ff18
+#define WDT_INT_CLEAR_SMC	0x8200ff18
 #define WDT_UNLOCK		0xf1d0dead
 #define WDT_DISABLE		0x0
 #define WDT_ENABLE		0x1
@@ -143,7 +143,7 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
 	struct keembay_wdt *wdt = dev_id;
 	struct arm_smccc_res res;
 
-	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
+	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
 	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
 	emergency_restart();
 
@@ -157,7 +157,7 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
 
 	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
 
-	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
+	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
 	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
 	watchdog_notify_pretimeout(&wdt->wdd);
 
-- 
2.17.1

[PATCH 10/10] watchdog: keembay: Typo corrections and other blank operations

[shruthi.sanil]

From: Shruthi Sanil <shruthi.sanil@intel.com>

Corrected typos, aligned the tabs and added new lines
wherever required for better readability

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Tested-by: Kris Pan <kris.pan@intel.com>
Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
---
 drivers/watchdog/keembay_wdt.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
index 0a8cf5b35699..6a2699021263 100644
--- a/drivers/watchdog/keembay_wdt.c
+++ b/drivers/watchdog/keembay_wdt.c
@@ -26,11 +26,14 @@
 #define WDT_TH_INT_MASK		BIT(8)
 #define WDT_TO_INT_MASK		BIT(9)
 #define WDT_INT_CLEAR_SMC	0x8200ff18
+
 #define WDT_UNLOCK		0xf1d0dead
 #define WDT_DISABLE		0x0
 #define WDT_ENABLE		0x1
+
 #define WDT_LOAD_MAX		U32_MAX
 #define WDT_LOAD_MIN		1
+
 #define WDT_TIMEOUT		5
 #define WDT_PRETIMEOUT		4
 
@@ -144,7 +147,7 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
 	struct arm_smccc_res res;
 
 	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
-	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
+	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-secure wdt timeout.\n");
 	emergency_restart();
 
 	return IRQ_HANDLED;
@@ -158,7 +161,7 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
 	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
 
 	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
-	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
+	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-secure wdt pre-timeout.\n");
 	watchdog_notify_pretimeout(&wdt->wdd);
 
 	return IRQ_HANDLED;
@@ -278,8 +281,8 @@ static const struct of_device_id keembay_wdt_match[] = {
 MODULE_DEVICE_TABLE(of, keembay_wdt_match);
 
 static struct platform_driver keembay_wdt_driver = {
-	.probe		= keembay_wdt_probe,
-	.driver		= {
+	.probe	= keembay_wdt_probe,
+	.driver	= {
 		.name		= "keembay_wdt",
 		.of_match_table	= keembay_wdt_match,
 		.pm		= &keembay_wdt_pm_ops,
-- 
2.17.1

Re: [PATCH 01/10] watchdog: keembay: Update WDT pre-timeout during the initialization

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:15PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> The pretimeout register has a default reset value. Hence
> when a smaller WDT timeout is set which would be lesser than the
> default pretimeout, the system behaves abnormally, starts
> triggering the pretimeout interrupt even when the WDT is
> not enabled, most of the times leading to system crash.
> Hence an update in the pre-timeout is also required for the
> default timeout that is being configured.
> 
> Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index 547d3fea33ff..f2f5c9fae29c 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -29,6 +29,7 @@
>  #define WDT_LOAD_MAX		U32_MAX
>  #define WDT_LOAD_MIN		1
>  #define WDT_TIMEOUT		5
> +#define WDT_PRETIMEOUT		4
>  
>  static unsigned int timeout = WDT_TIMEOUT;
>  module_param(timeout, int, 0);
> @@ -224,11 +225,13 @@ static int keembay_wdt_probe(struct platform_device *pdev)
>  	wdt->wdd.min_timeout	= WDT_LOAD_MIN;
>  	wdt->wdd.max_timeout	= WDT_LOAD_MAX / wdt->rate;
>  	wdt->wdd.timeout	= WDT_TIMEOUT;
> +	wdt->wdd.pretimeout	= WDT_PRETIMEOUT;
>  
>  	watchdog_set_drvdata(&wdt->wdd, wdt);
>  	watchdog_set_nowayout(&wdt->wdd, nowayout);
>  	watchdog_init_timeout(&wdt->wdd, timeout, dev);
>  	keembay_wdt_set_timeout(&wdt->wdd, wdt->wdd.timeout);
> +	keembay_wdt_set_pretimeout(&wdt->wdd, wdt->wdd.pretimeout);
>  
>  	ret = devm_watchdog_register_device(dev, &wdt->wdd);
>  	if (ret)
> -- 
> 2.17.1
> 

Re: [PATCH 02/10] watchdog: keembay: Upadate WDT pretimeout for every update in timeout

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:16PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> The pre-timeout value to be programmed to the register has to be
> calculated and updated for every change in the timeout value.
> Else the threshold time wouldn't be calculated to its
> corresponding timeout.
> 
> Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index f2f5c9fae29c..b2afeb4a60e3 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -109,6 +109,7 @@ static int keembay_wdt_set_timeout(struct watchdog_device *wdog, u32 t)
>  {
>  	wdog->timeout = t;
>  	keembay_wdt_set_timeout_reg(wdog);
> +	keembay_wdt_set_pretimeout_reg(wdog);
>  
>  	return 0;
>  }
> -- 
> 2.17.1
> 

Re: [PATCH 03/10] watchdog: keembay: Update pretimeout to zero in the TH ISR

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:17PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> The pretimeout has to be updated to zero during the ISR of the
> ThresHold interrupt. Else the TH interrupt would be triggerred for
> every tick until the timeout.
> 
> Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index b2afeb4a60e3..6053416b8d3d 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -154,6 +154,8 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
>  	struct keembay_wdt *wdt = dev_id;
>  	struct arm_smccc_res res;
>  
> +	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
> +
>  	arm_smccc_smc(WDT_ISR_CLEAR, WDT_ISR_MASK, 0, 0, 0, 0, 0, 0, &res);
>  	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
>  	watchdog_notify_pretimeout(&wdt->wdd);
> -- 
> 2.17.1
> 

Re: [PATCH 04/10] watchdog: keembay: Clear either the TO or TH interrupt bit

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:18PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> During the interrupt service routine of the TimeOut interrupt and
> the ThresHold interrupt, the respective interrupt clear bit
> have to be cleared and not both.
> 
> Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index 6053416b8d3d..f2a16c9933c3 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -23,7 +23,8 @@
>  #define TIM_WDOG_EN		0x8
>  #define TIM_SAFE		0xc
>  
> -#define WDT_ISR_MASK		GENMASK(9, 8)
> +#define WDT_TH_INT_MASK		BIT(8)
> +#define WDT_TO_INT_MASK		BIT(9)
>  #define WDT_ISR_CLEAR		0x8200ff18
>  #define WDT_UNLOCK		0xf1d0dead
>  #define WDT_LOAD_MAX		U32_MAX
> @@ -142,7 +143,7 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
>  	struct arm_smccc_res res;
>  
>  	keembay_wdt_writel(wdt, TIM_WATCHDOG, 1);
> -	arm_smccc_smc(WDT_ISR_CLEAR, WDT_ISR_MASK, 0, 0, 0, 0, 0, 0, &res);
> +	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
>  	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
>  	emergency_restart();
>  
> @@ -156,7 +157,7 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
>  
>  	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
>  
> -	arm_smccc_smc(WDT_ISR_CLEAR, WDT_ISR_MASK, 0, 0, 0, 0, 0, 0, &res);
> +	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
>  	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
>  	watchdog_notify_pretimeout(&wdt->wdd);
>  
> -- 
> 2.17.1
> 

Re: [PATCH 05/10] watchdog: keembay: Remove timeout update in the WDT start function

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:19PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> Removed set timeout from the start WDT function. There is a function
> defined to set the timeout. Hence no need to set the timeout again in
> start function as the timeout would have been already updated
> before calling the start/enable.
> 
> Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index f2a16c9933c3..039753b9932b 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -84,7 +84,6 @@ static int keembay_wdt_start(struct watchdog_device *wdog)
>  {
>  	struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
>  
> -	keembay_wdt_set_timeout_reg(wdog);
>  	keembay_wdt_writel(wdt, TIM_WDOG_EN, 1);
>  
>  	return 0;
> -- 
> 2.17.1
> 

Re: [PATCH 06/10] watchdog: keembay: Removed timeout update in the TO ISR

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:20PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> In the TO ISR removed updating the Timeout value because
> its not serving any purpose as the timer would have already expired
> and the system would be rebooting.
> 
> Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index 039753b9932b..dd192b8dff55 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -141,7 +141,6 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
>  	struct keembay_wdt *wdt = dev_id;
>  	struct arm_smccc_res res;
>  
> -	keembay_wdt_writel(wdt, TIM_WATCHDOG, 1);
>  	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
>  	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
>  	emergency_restart();
> -- 
> 2.17.1
> 

Re: [PATCH 07/10] watchdog: keembay: Update the check in keembay_wdt_resume()

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:21PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> Corrected the typo in the function keembay_wdt_resume, we need to
> enable the WDT if it is disabled/not active.
> 
> Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay Soc")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
> ---
>  drivers/watchdog/keembay_wdt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index dd192b8dff55..10896415f8c7 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -260,7 +260,7 @@ static int __maybe_unused keembay_wdt_resume(struct device *dev)
>  {
>  	struct keembay_wdt *wdt = dev_get_drvdata(dev);
>  
> -	if (watchdog_active(&wdt->wdd))
> +	if (!watchdog_active(&wdt->wdd))

Have you tested this ? "watchdog_active" refers to the watchdog core state.
Your code now keeps the watchdog stopped after resume if it was running before,
and starts it if it wasn't. Please run through a suspend/resume cycle with
watchdog disabled and see what happens.

Guenter

>  		return keembay_wdt_start(&wdt->wdd);
>  
>  	return 0;
> -- 
> 2.17.1
> 

Re: [PATCH 08/10] watchdog: keembay: MACRO for WDT enable and disable values

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:22PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> Introduced MACRO's for WDT enable and disable values for better readability
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index 10896415f8c7..63a7c5d719a3 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -27,6 +27,8 @@
>  #define WDT_TO_INT_MASK		BIT(9)
>  #define WDT_ISR_CLEAR		0x8200ff18
>  #define WDT_UNLOCK		0xf1d0dead
> +#define WDT_DISABLE		0x0
> +#define WDT_ENABLE		0x1
>  #define WDT_LOAD_MAX		U32_MAX
>  #define WDT_LOAD_MIN		1
>  #define WDT_TIMEOUT		5
> @@ -84,7 +86,7 @@ static int keembay_wdt_start(struct watchdog_device *wdog)
>  {
>  	struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
>  
> -	keembay_wdt_writel(wdt, TIM_WDOG_EN, 1);
> +	keembay_wdt_writel(wdt, TIM_WDOG_EN, WDT_ENABLE);
>  
>  	return 0;
>  }
> @@ -93,7 +95,7 @@ static int keembay_wdt_stop(struct watchdog_device *wdog)
>  {
>  	struct keembay_wdt *wdt = watchdog_get_drvdata(wdog);
>  
> -	keembay_wdt_writel(wdt, TIM_WDOG_EN, 0);
> +	keembay_wdt_writel(wdt, TIM_WDOG_EN, WDT_DISABLE);
>  
>  	return 0;
>  }
> -- 
> 2.17.1
> 

Re: [PATCH 09/10] watchdog: keembay: WDT SMC handler MACRO name update

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:23PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> Updated the WDT SMC handler MACRO name to make it clear that its
> a ARM SMC handler that helps in clearing the WDT interrupt bit.
> 
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index 63a7c5d719a3..0a8cf5b35699 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -25,7 +25,7 @@
>  
>  #define WDT_TH_INT_MASK		BIT(8)
>  #define WDT_TO_INT_MASK		BIT(9)
> -#define WDT_ISR_CLEAR		0x8200ff18
> +#define WDT_INT_CLEAR_SMC	0x8200ff18
>  #define WDT_UNLOCK		0xf1d0dead
>  #define WDT_DISABLE		0x0
>  #define WDT_ENABLE		0x1
> @@ -143,7 +143,7 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
>  	struct keembay_wdt *wdt = dev_id;
>  	struct arm_smccc_res res;
>  
> -	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
> +	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
>  	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
>  	emergency_restart();
>  
> @@ -157,7 +157,7 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
>  
>  	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
>  
> -	arm_smccc_smc(WDT_ISR_CLEAR, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
> +	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
>  	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
>  	watchdog_notify_pretimeout(&wdt->wdd);
>  
> -- 
> 2.17.1
> 

Re: [PATCH 10/10] watchdog: keembay: Typo corrections and other blank operations

[Guenter Roeck]

On Wed, May 12, 2021 at 02:17:24PM +0530, shruthi.sanil@intel.com wrote:
> From: Shruthi Sanil <shruthi.sanil@intel.com>
> 
> Corrected typos, aligned the tabs and added new lines
> wherever required for better readability
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Tested-by: Kris Pan <kris.pan@intel.com>
> Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>

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

> ---
>  drivers/watchdog/keembay_wdt.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/keembay_wdt.c b/drivers/watchdog/keembay_wdt.c
> index 0a8cf5b35699..6a2699021263 100644
> --- a/drivers/watchdog/keembay_wdt.c
> +++ b/drivers/watchdog/keembay_wdt.c
> @@ -26,11 +26,14 @@
>  #define WDT_TH_INT_MASK		BIT(8)
>  #define WDT_TO_INT_MASK		BIT(9)
>  #define WDT_INT_CLEAR_SMC	0x8200ff18
> +
>  #define WDT_UNLOCK		0xf1d0dead
>  #define WDT_DISABLE		0x0
>  #define WDT_ENABLE		0x1
> +
>  #define WDT_LOAD_MAX		U32_MAX
>  #define WDT_LOAD_MIN		1
> +
>  #define WDT_TIMEOUT		5
>  #define WDT_PRETIMEOUT		4
>  
> @@ -144,7 +147,7 @@ static irqreturn_t keembay_wdt_to_isr(int irq, void *dev_id)
>  	struct arm_smccc_res res;
>  
>  	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TO_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
> -	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt timeout.\n");
> +	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-secure wdt timeout.\n");
>  	emergency_restart();
>  
>  	return IRQ_HANDLED;
> @@ -158,7 +161,7 @@ static irqreturn_t keembay_wdt_th_isr(int irq, void *dev_id)
>  	keembay_wdt_set_pretimeout(&wdt->wdd, 0x0);
>  
>  	arm_smccc_smc(WDT_INT_CLEAR_SMC, WDT_TH_INT_MASK, 0, 0, 0, 0, 0, 0, &res);
> -	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-sec wdt pre-timeout.\n");
> +	dev_crit(wdt->wdd.parent, "Intel Keem Bay non-secure wdt pre-timeout.\n");
>  	watchdog_notify_pretimeout(&wdt->wdd);
>  
>  	return IRQ_HANDLED;
> @@ -278,8 +281,8 @@ static const struct of_device_id keembay_wdt_match[] = {
>  MODULE_DEVICE_TABLE(of, keembay_wdt_match);
>  
>  static struct platform_driver keembay_wdt_driver = {
> -	.probe		= keembay_wdt_probe,
> -	.driver		= {
> +	.probe	= keembay_wdt_probe,
> +	.driver	= {
>  		.name		= "keembay_wdt",
>  		.of_match_table	= keembay_wdt_match,
>  		.pm		= &keembay_wdt_pm_ops,
> -- 
> 2.17.1
> 

RE: [PATCH 07/10] watchdog: keembay: Update the check in keembay_wdt_resume()

[Sanil, Shruthi]

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Wednesday, May 12, 2021 7:32 PM
> To: Sanil, Shruthi <shruthi.sanil@intel.com>
> Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; linux-
> kernel@vger.kernel.org; andriy.shevchenko@linux.intel.com;
> kris.pan@linux.intel.com; mgross@linux.intel.com; Thokala, Srikanth
> <srikanth.thokala@intel.com>; Raja Subramanian, Lakshmi Bai
> <lakshmi.bai.raja.subramanian@intel.com>; Sangannavar, Mallikarjunappa
> <mallikarjunappa.sangannavar@intel.com>
> Subject: Re: [PATCH 07/10] watchdog: keembay: Update the check in
> keembay_wdt_resume()
> 
> On Wed, May 12, 2021 at 02:17:21PM +0530, shruthi.sanil@intel.com wrote:
> > From: Shruthi Sanil <shruthi.sanil@intel.com>
> >
> > Corrected the typo in the function keembay_wdt_resume, we need to
> > enable the WDT if it is disabled/not active.
> >
> > Fixes: fa0f8d51e90d ("watchdog: Add watchdog driver for Intel Keembay
> > Soc")
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Tested-by: Kris Pan <kris.pan@intel.com>
> > Signed-off-by: Shruthi Sanil <shruthi.sanil@intel.com>
> > ---
> >  drivers/watchdog/keembay_wdt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/watchdog/keembay_wdt.c
> > b/drivers/watchdog/keembay_wdt.c index dd192b8dff55..10896415f8c7
> > 100644
> > --- a/drivers/watchdog/keembay_wdt.c
> > +++ b/drivers/watchdog/keembay_wdt.c
> > @@ -260,7 +260,7 @@ static int __maybe_unused
> > keembay_wdt_resume(struct device *dev)  {
> >  	struct keembay_wdt *wdt = dev_get_drvdata(dev);
> >
> > -	if (watchdog_active(&wdt->wdd))
> > +	if (!watchdog_active(&wdt->wdd))
> 
> Have you tested this ? "watchdog_active" refers to the watchdog core state.
> Your code now keeps the watchdog stopped after resume if it was running
> before, and starts it if it wasn't. Please run through a suspend/resume cycle
> with watchdog disabled and see what happens.
> 
> Guenter

I had understood it wrongly. I was assuming that watchdog_active refers to the actual state of the HW. Hence I made that change.
But since watchdog_active refers to the state of the core, the change that I have done is incorrect. Hence I'll drop this patch.

Thanks
Shruthi

> 
> >  		return keembay_wdt_start(&wdt->wdd);
> >
> >  	return 0;
> > --
> > 2.17.1
> >