[PATCH v2 0/7] watchdog: imx7ulp_wdt: update i.MX7ULP WDOG timer driver

[PATCH v2 0/7] watchdog: imx7ulp_wdt: update i.MX7ULP WDOG timer driver

[Alice Guo (OSS)]

From: Alice Guo <alice.guo@nxp.com>

Alice Guo (1):
  watchdog: imx93: add watchdog timer on imx93

Anson Huang (1):
  watchdog: imx7ulp: Move suspend/resume to noirq phase

Jacky Bai (1):
  watchdog: imx7ulp: Add explict memory barrier for unlock sequence

Jason Liu (1):
  watchdog: imx7ulp_wdt: init wdog when it was active

Ye Li (3):
  watchdog: imx7ulp_wdt: Check CMD32EN in wdog init
  watchdog: imx7ulp_wdt: Fix RCS timeout issue
  watchdog: imx7ulp_wdt: Handle wdog reconfigure failure

 drivers/watchdog/imx7ulp_wdt.c | 212 ++++++++++++++++++++++++++-------
 1 file changed, 168 insertions(+), 44 deletions(-)

-- 
2.17.1

[PATCH v2 1/7] watchdog: imx7ulp: Move suspend/resume to noirq phase

[Alice Guo (OSS)]

From: Anson Huang <Anson.Huang@nxp.com>

The i.MX7ULP's watchdog is enabled by default when out of reset, so the
resume callback which is to disable watchdog should be called earlier
to avoid unexpected timeout, move suspend/resume callback to noirq phase.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Tested-by: Peter Chen <peter.chen@nxp.com>
Tested-by: Li Jun <jun.li@nxp.com>
---

Changes for v2:
 - none

 drivers/watchdog/imx7ulp_wdt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index 922b60374295..014f497ea0dc 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -255,7 +255,7 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
 	return devm_watchdog_register_device(dev, wdog);
 }
 
-static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev)
+static int __maybe_unused imx7ulp_wdt_suspend_noirq(struct device *dev)
 {
 	struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
 
@@ -267,7 +267,7 @@ static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused imx7ulp_wdt_resume(struct device *dev)
+static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
 {
 	struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
 	u32 timeout = imx7ulp_wdt->wdd.timeout * WDOG_CLOCK_RATE;
@@ -286,8 +286,10 @@ static int __maybe_unused imx7ulp_wdt_resume(struct device *dev)
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(imx7ulp_wdt_pm_ops, imx7ulp_wdt_suspend,
-			 imx7ulp_wdt_resume);
+static const struct dev_pm_ops imx7ulp_wdt_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx7ulp_wdt_suspend_noirq,
+				      imx7ulp_wdt_resume_noirq)
+};
 
 static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
 	{ .compatible = "fsl,imx7ulp-wdt", },
-- 
2.17.1

[PATCH v2 2/7] watchdog: imx7ulp: Add explict memory barrier for unlock sequence

[Alice Guo (OSS)]

From: Jacky Bai <ping.bai@nxp.com>

When reconfiguring the WDOG Timer of i.MX7ULP, there is a certain
probability causes it to reset. The reason is that the CMD32EN of the
WDOG Timer of i.MX7ULP is disabled in bootloader. The unlock sequence
are two 16-bit writes to the CNT register within 16 bus clocks. Adding
mb() is to guarantee that two 16-bit writes are finished within 16 bus
clocks. Memory barriers cannot be added between these two 16-bit writes
so that writel_relaxed is used.

Suggested-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
---

Changes for v2:
 - add the reason why memory barriers are added for unlock sequence in commit log

 drivers/watchdog/imx7ulp_wdt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index 014f497ea0dc..b8ac0cb04d2f 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -179,9 +179,13 @@ static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
 	int ret;
 
 	local_irq_disable();
+
+	mb();
 	/* unlock the wdog for reconfiguration */
 	writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
 	writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
+	mb();
+
 	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
 	if (ret)
 		goto init_out;
-- 
2.17.1

[PATCH v2 3/7] watchdog: imx7ulp_wdt: Check CMD32EN in wdog init

[Alice Guo (OSS)]

From: Ye Li <ye.li@nxp.com>

When bootloader has enabled the CMD32EN bit, switch to use 32bits
unlock command to unlock the CS register. Using 32bits command will
help on avoiding 16 bus cycle window violation for two 16 bits
commands.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Acked-by: Jason Liu <jason.hui.liu@nxp.com>
---

Changes for v2:
 - none

 drivers/watchdog/imx7ulp_wdt.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index b8ac0cb04d2f..a0f6b8cea78f 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -180,11 +180,16 @@ static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
 
 	local_irq_disable();
 
-	mb();
-	/* unlock the wdog for reconfiguration */
-	writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
-	writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
-	mb();
+	val = readl(base + WDOG_CS);
+	if (val & WDOG_CS_CMD32EN) {
+		writel(UNLOCK, base + WDOG_CNT);
+	} else {
+		mb();
+		/* unlock the wdog for reconfiguration */
+		writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
+		writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
+		mb();
+	}
 
 	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
 	if (ret)
-- 
2.17.1

[PATCH v2 4/7] watchdog: imx7ulp_wdt: Fix RCS timeout issue

[Alice Guo (OSS)]

From: Ye Li <ye.li@nxp.com>

According to measure on i.MX7ULP and i.MX8ULP, the RCS done needs
about 3400us and 6700us respectively. So current 20us timeout is
not enough. When reconfiguring is on-going, unlock and configure CS
will lead to unknown result.

Increase the wait timeout value to 10ms and check the return value
of RCS wait to fix the issue

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Acked-by: Jason Liu <jason.hui.liu@nxp.com>
---

Changes for v2:
 - none

 drivers/watchdog/imx7ulp_wdt.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index a0f6b8cea78f..12715c248688 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -39,7 +39,7 @@
 #define DEFAULT_TIMEOUT	60
 #define MAX_TIMEOUT	128
 #define WDOG_CLOCK_RATE	1000
-#define WDOG_WAIT_TIMEOUT	20
+#define WDOG_WAIT_TIMEOUT	10000
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0000);
@@ -80,7 +80,7 @@ static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
 		writel(val | WDOG_CS_EN, wdt->base + WDOG_CS);
 	else
 		writel(val & ~WDOG_CS_EN, wdt->base + WDOG_CS);
-	imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
+	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
 
 enable_out:
 	local_irq_enable();
@@ -127,7 +127,9 @@ static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
 	if (ret)
 		goto timeout_out;
 	writel(val, wdt->base + WDOG_TOVAL);
-	imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
+	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
+	if (ret)
+		goto timeout_out;
 
 	wdog->timeout = timeout;
 
-- 
2.17.1

[PATCH v2 5/7] watchdog: imx7ulp_wdt: Handle wdog reconfigure failure

[Alice Guo (OSS)]

From: Ye Li <ye.li@nxp.com>

Current driver may meet reconfigure failure caused by below reasons:

1. The wdog on iMX7ULP has different behavior after RCS valid. It needs
   to wait more than 2.5 wdog clock for clock sync before next
   reconfiguration, while imx8ulp wdog does not need such delay.

2. After unlock, there is 128 bus clock window opened for reconfiguration,
   but on iMX8ULP, the HW can't guarantee the latency. So it is possible
   the window is closed before the writing arrives to wdog.

3. If the PRES is enabled, the RCS valid time becomes x256 to the time
   of PRES disabled. It is about 1715ms on iMX8ULP. So We have to increase
   the RCS timeout and can't wait it in IRQ disabled.

The patch updates the driver to handle failures

1. Using different wait for unlock and RCS. Unlock valid time is very short
   and only related to bus clock. It must be in IRQ disabled to avoid
   being interrupted in 128 clock window. But for RCS time, it is longer
   and ok for IRQ enabled.

2. Add retry for any reconfigure failure with default 5 times.

3. Add "fsl,imx8ulp-wdt" compatile string for iMX8ULP and afterwards
   platform which don't need more 2.5 wdog clock after RCS valid.
   For imx7ulp, add post delay of 2.5 clock after RCS valid.

Signed-off-by: Ye Li <ye.li@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
---

Changes for v2:
 - the wait timeout value of the RCS is 10ms, so use the sleep_us of
   readl_poll_timeout in imx7ulp_wdt_wait_rcs to avoid a 10ms hot wait

 drivers/watchdog/imx7ulp_wdt.c | 163 ++++++++++++++++++++++++++-------
 1 file changed, 129 insertions(+), 34 deletions(-)

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index 12715c248688..0cafa86fff7f 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -14,7 +14,9 @@
 #include <linux/watchdog.h>
 
 #define WDOG_CS			0x0
+#define WDOG_CS_FLG		BIT(14)
 #define WDOG_CS_CMD32EN		BIT(13)
+#define WDOG_CS_PRES		BIT(12)
 #define WDOG_CS_ULK		BIT(11)
 #define WDOG_CS_RCS		BIT(10)
 #define LPO_CLK			0x1
@@ -39,7 +41,11 @@
 #define DEFAULT_TIMEOUT	60
 #define MAX_TIMEOUT	128
 #define WDOG_CLOCK_RATE	1000
-#define WDOG_WAIT_TIMEOUT	10000
+#define WDOG_ULK_WAIT_TIMEOUT	1000
+#define WDOG_RCS_WAIT_TIMEOUT	10000
+#define WDOG_RCS_POST_WAIT 3000
+
+#define RETRY_MAX 5
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0000);
@@ -50,40 +56,82 @@ struct imx7ulp_wdt_device {
 	struct watchdog_device wdd;
 	void __iomem *base;
 	struct clk *clk;
+	bool post_rcs_wait;
 };
 
-static int imx7ulp_wdt_wait(void __iomem *base, u32 mask)
+static int imx7ulp_wdt_wait_ulk(void __iomem *base)
 {
 	u32 val = readl(base + WDOG_CS);
 
-	if (!(val & mask) && readl_poll_timeout_atomic(base + WDOG_CS, val,
-						       val & mask, 0,
-						       WDOG_WAIT_TIMEOUT))
+	if (!(val & WDOG_CS_ULK) &&
+	    readl_poll_timeout_atomic(base + WDOG_CS, val,
+				      val & WDOG_CS_ULK, 0,
+				      WDOG_ULK_WAIT_TIMEOUT))
 		return -ETIMEDOUT;
 
 	return 0;
 }
 
-static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
+static int imx7ulp_wdt_wait_rcs(struct imx7ulp_wdt_device *wdt)
 {
-	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
+	int ret = 0;
+	u32 val = readl(wdt->base + WDOG_CS);
+	u64 timeout = (val & WDOG_CS_PRES) ?
+		WDOG_RCS_WAIT_TIMEOUT * 256 : WDOG_RCS_WAIT_TIMEOUT;
+	unsigned long wait_min = (val & WDOG_CS_PRES) ?
+		WDOG_RCS_POST_WAIT * 256 : WDOG_RCS_POST_WAIT;
 
+	if (!(val & WDOG_CS_RCS) &&
+	    readl_poll_timeout(wdt->base + WDOG_CS, val, val & WDOG_CS_RCS, 100,
+			       timeout))
+		ret = -ETIMEDOUT;
+
+	/* Wait 2.5 clocks after RCS done */
+	if (wdt->post_rcs_wait)
+		usleep_range(wait_min, wait_min + 2000);
+
+	return ret;
+}
+
+static int _imx7ulp_wdt_enable(struct imx7ulp_wdt_device *wdt, bool enable)
+{
 	u32 val = readl(wdt->base + WDOG_CS);
 	int ret;
 
 	local_irq_disable();
 	writel(UNLOCK, wdt->base + WDOG_CNT);
-	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_ULK);
+	ret = imx7ulp_wdt_wait_ulk(wdt->base);
 	if (ret)
 		goto enable_out;
 	if (enable)
 		writel(val | WDOG_CS_EN, wdt->base + WDOG_CS);
 	else
 		writel(val & ~WDOG_CS_EN, wdt->base + WDOG_CS);
-	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
+
+	local_irq_enable();
+	ret = imx7ulp_wdt_wait_rcs(wdt);
+
+	return ret;
 
 enable_out:
 	local_irq_enable();
+	return ret;
+}
+
+static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
+{
+	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
+	int ret;
+	u32 val;
+	u32 loop = RETRY_MAX;
+
+	do {
+		ret = _imx7ulp_wdt_enable(wdt, enable);
+		val = readl(wdt->base + WDOG_CS);
+	} while (--loop > 0 && ((!!(val & WDOG_CS_EN)) != enable || ret));
+
+	if (loop == 0)
+		return -EBUSY;
 
 	return ret;
 }
@@ -114,28 +162,44 @@ static int imx7ulp_wdt_stop(struct watchdog_device *wdog)
 	return imx7ulp_wdt_enable(wdog, false);
 }
 
-static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
-				   unsigned int timeout)
+static int _imx7ulp_wdt_set_timeout(struct imx7ulp_wdt_device *wdt,
+				   unsigned int toval)
 {
-	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
-	u32 val = WDOG_CLOCK_RATE * timeout;
 	int ret;
 
 	local_irq_disable();
 	writel(UNLOCK, wdt->base + WDOG_CNT);
-	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_ULK);
+	ret = imx7ulp_wdt_wait_ulk(wdt->base);
 	if (ret)
 		goto timeout_out;
-	writel(val, wdt->base + WDOG_TOVAL);
-	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
-	if (ret)
-		goto timeout_out;
-
-	wdog->timeout = timeout;
+	writel(toval, wdt->base + WDOG_TOVAL);
+	local_irq_enable();
+	ret = imx7ulp_wdt_wait_rcs(wdt);
+	return ret;
 
 timeout_out:
 	local_irq_enable();
+	return ret;
+}
 
+static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
+				   unsigned int timeout)
+{
+	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
+	u32 toval = WDOG_CLOCK_RATE * timeout;
+	u32 val;
+	int ret;
+	u32 loop = RETRY_MAX;
+
+	do {
+		ret = _imx7ulp_wdt_set_timeout(wdt, toval);
+		val = readl(wdt->base + WDOG_TOVAL);
+	} while (--loop > 0 && (val != toval || ret));
+
+	if (loop == 0)
+		return -EBUSY;
+
+	wdog->timeout = timeout;
 	return ret;
 }
 
@@ -175,38 +239,59 @@ static const struct watchdog_info imx7ulp_wdt_info = {
 		    WDIOF_MAGICCLOSE,
 };
 
-static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
+static int _imx7ulp_wdt_init(struct imx7ulp_wdt_device *wdt, unsigned int timeout, unsigned int cs)
 {
 	u32 val;
 	int ret;
 
 	local_irq_disable();
 
-	val = readl(base + WDOG_CS);
+	val = readl(wdt->base + WDOG_CS);
 	if (val & WDOG_CS_CMD32EN) {
-		writel(UNLOCK, base + WDOG_CNT);
+		writel(UNLOCK, wdt->base + WDOG_CNT);
 	} else {
 		mb();
 		/* unlock the wdog for reconfiguration */
-		writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
-		writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
+		writel_relaxed(UNLOCK_SEQ0, wdt->base + WDOG_CNT);
+		writel_relaxed(UNLOCK_SEQ1, wdt->base + WDOG_CNT);
 		mb();
 	}
 
-	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
+	ret = imx7ulp_wdt_wait_ulk(wdt->base);
 	if (ret)
 		goto init_out;
 
 	/* set an initial timeout value in TOVAL */
-	writel(timeout, base + WDOG_TOVAL);
-	/* enable 32bit command sequence and reconfigure */
-	val = WDOG_CS_CMD32EN | WDOG_CS_CLK | WDOG_CS_UPDATE |
-	      WDOG_CS_WAIT | WDOG_CS_STOP;
-	writel(val, base + WDOG_CS);
-	imx7ulp_wdt_wait(base, WDOG_CS_RCS);
+	writel(timeout, wdt->base + WDOG_TOVAL);
+	writel(cs, wdt->base + WDOG_CS);
+	local_irq_enable();
+	ret = imx7ulp_wdt_wait_rcs(wdt);
+
+	return ret;
 
 init_out:
 	local_irq_enable();
+	return ret;
+}
+
+static int imx7ulp_wdt_init(struct imx7ulp_wdt_device *wdt, unsigned int timeout)
+{
+	/* enable 32bit command sequence and reconfigure */
+	u32 val = WDOG_CS_CMD32EN | WDOG_CS_CLK | WDOG_CS_UPDATE |
+		  WDOG_CS_WAIT | WDOG_CS_STOP;
+	u32 cs, toval;
+	int ret;
+	u32 loop = RETRY_MAX;
+
+	do {
+		ret = _imx7ulp_wdt_init(wdt, timeout, val);
+		toval = readl(wdt->base + WDOG_TOVAL);
+		cs = readl(wdt->base + WDOG_CS);
+		cs &= ~(WDOG_CS_FLG | WDOG_CS_ULK | WDOG_CS_RCS);
+	} while (--loop > 0 && (cs != val || toval != timeout || ret));
+
+	if (loop == 0)
+		return -EBUSY;
 
 	return ret;
 }
@@ -239,6 +324,15 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
 		return PTR_ERR(imx7ulp_wdt->clk);
 	}
 
+	imx7ulp_wdt->post_rcs_wait = true;
+	if (of_device_is_compatible(dev->of_node,
+				    "fsl,imx8ulp-wdt")) {
+		dev_info(dev, "imx8ulp wdt probe\n");
+		imx7ulp_wdt->post_rcs_wait = false;
+	} else {
+		dev_info(dev, "imx7ulp wdt probe\n");
+	}
+
 	ret = clk_prepare_enable(imx7ulp_wdt->clk);
 	if (ret)
 		return ret;
@@ -259,7 +353,7 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
 	watchdog_stop_on_reboot(wdog);
 	watchdog_stop_on_unregister(wdog);
 	watchdog_set_drvdata(wdog, imx7ulp_wdt);
-	ret = imx7ulp_wdt_init(imx7ulp_wdt->base, wdog->timeout * WDOG_CLOCK_RATE);
+	ret = imx7ulp_wdt_init(imx7ulp_wdt, wdog->timeout * WDOG_CLOCK_RATE);
 	if (ret)
 		return ret;
 
@@ -289,7 +383,7 @@ static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
 		return ret;
 
 	if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
-		imx7ulp_wdt_init(imx7ulp_wdt->base, timeout);
+		imx7ulp_wdt_init(imx7ulp_wdt, timeout);
 
 	if (watchdog_active(&imx7ulp_wdt->wdd))
 		imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
@@ -303,6 +397,7 @@ static const struct dev_pm_ops imx7ulp_wdt_pm_ops = {
 };
 
 static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
+	{ .compatible = "fsl,imx8ulp-wdt", },
 	{ .compatible = "fsl,imx7ulp-wdt", },
 	{ /* sentinel */ }
 };
-- 
2.17.1

[PATCH v2 6/7] watchdog: imx7ulp_wdt: init wdog when it was active

[Alice Guo (OSS)]

From: Jason Liu <jason.hui.liu@nxp.com>

Paired with suspend, we can only init wdog again when it was active
and ping it once to avoid the watchdog timeout after it resumed.

Signed-off-by: Jason Liu <jason.hui.liu@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Tested-by: Jacky Bai <ping.bai@nxp.com>
---

Changes for v2:
 - none

 drivers/watchdog/imx7ulp_wdt.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index 0cafa86fff7f..dee02c2a52c9 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -136,13 +136,6 @@ static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
 	return ret;
 }
 
-static bool imx7ulp_wdt_is_enabled(void __iomem *base)
-{
-	u32 val = readl(base + WDOG_CS);
-
-	return val & WDOG_CS_EN;
-}
-
 static int imx7ulp_wdt_ping(struct watchdog_device *wdog)
 {
 	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
@@ -382,11 +375,11 @@ static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
 	if (ret)
 		return ret;
 
-	if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
+	if (watchdog_active(&imx7ulp_wdt->wdd)) {
 		imx7ulp_wdt_init(imx7ulp_wdt, timeout);
-
-	if (watchdog_active(&imx7ulp_wdt->wdd))
 		imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
+		imx7ulp_wdt_ping(&imx7ulp_wdt->wdd);
+	}
 
 	return 0;
 }
-- 
2.17.1

[PATCH v2 7/7] watchdog: imx93: add watchdog timer on imx93

[Alice Guo (OSS)]

From: Alice Guo <alice.guo@nxp.com>

The WDOG clocks are sourced from lpo_clk, and lpo_clk is the fixed
32KHz. TOVAL contains the 16-bit value used to set the timeout period of
the watchdog. When the timeout period exceeds 2 seconds, the value
written to the TOVAL register is larger than 16-bit can represent.
Enabling watchdog prescaler can solve this problem.

Two points need to be aware of:
1. watchdog prescaler enables a fixed 256 pre-scaling of watchdog
counter reference clock
2. reconfiguration takes about 55ms on imx93

Reviewed-by: Jacky Bai <ping.bai@nxp.com>
Signed-off-by: Alice Guo <alice.guo@nxp.com>
---

Changes for v2:
 - none

 drivers/watchdog/imx7ulp_wdt.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
index dee02c2a52c9..2897902090b3 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -9,6 +9,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/reboot.h>
 #include <linux/watchdog.h>
@@ -52,11 +53,17 @@ module_param(nowayout, bool, 0000);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+struct imx_wdt_hw_feature {
+	bool prescaler_enable;
+	u32 wdog_clock_rate;
+};
+
 struct imx7ulp_wdt_device {
 	struct watchdog_device wdd;
 	void __iomem *base;
 	struct clk *clk;
 	bool post_rcs_wait;
+	const struct imx_wdt_hw_feature *hw;
 };
 
 static int imx7ulp_wdt_wait_ulk(void __iomem *base)
@@ -179,7 +186,7 @@ static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
 				   unsigned int timeout)
 {
 	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
-	u32 toval = WDOG_CLOCK_RATE * timeout;
+	u32 toval = wdt->hw->wdog_clock_rate * timeout;
 	u32 val;
 	int ret;
 	u32 loop = RETRY_MAX;
@@ -276,6 +283,9 @@ static int imx7ulp_wdt_init(struct imx7ulp_wdt_device *wdt, unsigned int timeout
 	int ret;
 	u32 loop = RETRY_MAX;
 
+	if (wdt->hw->prescaler_enable)
+		val |= WDOG_CS_PRES;
+
 	do {
 		ret = _imx7ulp_wdt_init(wdt, timeout, val);
 		toval = readl(wdt->base + WDOG_TOVAL);
@@ -346,7 +356,9 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
 	watchdog_stop_on_reboot(wdog);
 	watchdog_stop_on_unregister(wdog);
 	watchdog_set_drvdata(wdog, imx7ulp_wdt);
-	ret = imx7ulp_wdt_init(imx7ulp_wdt, wdog->timeout * WDOG_CLOCK_RATE);
+
+	imx7ulp_wdt->hw = of_device_get_match_data(dev);
+	ret = imx7ulp_wdt_init(imx7ulp_wdt, wdog->timeout * imx7ulp_wdt->hw->wdog_clock_rate);
 	if (ret)
 		return ret;
 
@@ -368,7 +380,7 @@ static int __maybe_unused imx7ulp_wdt_suspend_noirq(struct device *dev)
 static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
 {
 	struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
-	u32 timeout = imx7ulp_wdt->wdd.timeout * WDOG_CLOCK_RATE;
+	u32 timeout = imx7ulp_wdt->wdd.timeout * imx7ulp_wdt->hw->wdog_clock_rate;
 	int ret;
 
 	ret = clk_prepare_enable(imx7ulp_wdt->clk);
@@ -389,9 +401,20 @@ static const struct dev_pm_ops imx7ulp_wdt_pm_ops = {
 				      imx7ulp_wdt_resume_noirq)
 };
 
+static const struct imx_wdt_hw_feature imx7ulp_wdt_hw = {
+	.prescaler_enable = false,
+	.wdog_clock_rate = 1000,
+};
+
+static const struct imx_wdt_hw_feature imx93_wdt_hw = {
+	.prescaler_enable = true,
+	.wdog_clock_rate = 125,
+};
+
 static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
-	{ .compatible = "fsl,imx8ulp-wdt", },
-	{ .compatible = "fsl,imx7ulp-wdt", },
+	{ .compatible = "fsl,imx8ulp-wdt", .data = &imx7ulp_wdt_hw, },
+	{ .compatible = "fsl,imx7ulp-wdt", .data = &imx7ulp_wdt_hw, },
+	{ .compatible = "fsl,imx93-wdt", .data = &imx93_wdt_hw, },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx7ulp_wdt_dt_ids);
-- 
2.17.1

RE: [PATCH v2 0/7] watchdog: imx7ulp_wdt: update i.MX7ULP WDOG timer driver

[Alice Guo (OSS)]

A friendly ping...

Best Regards,
Alice Guo

> -----Original Message-----
> From: Alice Guo (OSS) <alice.guo@oss.nxp.com>
> Sent: Thursday, August 25, 2022 4:33 PM
> To: m.felsch@pengutronix.de; linux@roeck-us.net; wim@linux-watchdog.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; festevam@gmail.com
> Cc: kernel@pengutronix.de; dl-linux-imx <linux-imx@nxp.com>;
> linux-watchdog@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: [PATCH v2 0/7] watchdog: imx7ulp_wdt: update i.MX7ULP WDOG
> timer driver
> 
> From: Alice Guo <alice.guo@nxp.com>
> 
> Alice Guo (1):
>   watchdog: imx93: add watchdog timer on imx93
> 
> Anson Huang (1):
>   watchdog: imx7ulp: Move suspend/resume to noirq phase
> 
> Jacky Bai (1):
>   watchdog: imx7ulp: Add explict memory barrier for unlock sequence
> 
> Jason Liu (1):
>   watchdog: imx7ulp_wdt: init wdog when it was active
> 
> Ye Li (3):
>   watchdog: imx7ulp_wdt: Check CMD32EN in wdog init
>   watchdog: imx7ulp_wdt: Fix RCS timeout issue
>   watchdog: imx7ulp_wdt: Handle wdog reconfigure failure
> 
>  drivers/watchdog/imx7ulp_wdt.c | 212
> ++++++++++++++++++++++++++-------
>  1 file changed, 168 insertions(+), 44 deletions(-)
> 
> --
> 2.17.1

Re: [PATCH v2 1/7] watchdog: imx7ulp: Move suspend/resume to noirq phase

[Guenter Roeck]

On Thu, Aug 25, 2022 at 04:32:50PM +0800, Alice Guo (OSS) wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> The i.MX7ULP's watchdog is enabled by default when out of reset, so the
> resume callback which is to disable watchdog should be called earlier
> to avoid unexpected timeout, move suspend/resume callback to noirq phase.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Tested-by: Peter Chen <peter.chen@nxp.com>
> Tested-by: Li Jun <jun.li@nxp.com>

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

> ---
> 
> Changes for v2:
>  - none
> 
>  drivers/watchdog/imx7ulp_wdt.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index 922b60374295..014f497ea0dc 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -255,7 +255,7 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
>  	return devm_watchdog_register_device(dev, wdog);
>  }
>  
> -static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev)
> +static int __maybe_unused imx7ulp_wdt_suspend_noirq(struct device *dev)
>  {
>  	struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
>  
> @@ -267,7 +267,7 @@ static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int __maybe_unused imx7ulp_wdt_resume(struct device *dev)
> +static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
>  {
>  	struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
>  	u32 timeout = imx7ulp_wdt->wdd.timeout * WDOG_CLOCK_RATE;
> @@ -286,8 +286,10 @@ static int __maybe_unused imx7ulp_wdt_resume(struct device *dev)
>  	return 0;
>  }
>  
> -static SIMPLE_DEV_PM_OPS(imx7ulp_wdt_pm_ops, imx7ulp_wdt_suspend,
> -			 imx7ulp_wdt_resume);
> +static const struct dev_pm_ops imx7ulp_wdt_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx7ulp_wdt_suspend_noirq,
> +				      imx7ulp_wdt_resume_noirq)
> +};
>  
>  static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
>  	{ .compatible = "fsl,imx7ulp-wdt", },
> -- 
> 2.17.1
> 

Re: [PATCH v2 2/7] watchdog: imx7ulp: Add explict memory barrier for unlock sequence

[Guenter Roeck]

On Thu, Aug 25, 2022 at 04:32:51PM +0800, Alice Guo (OSS) wrote:
> From: Jacky Bai <ping.bai@nxp.com>
> 
> When reconfiguring the WDOG Timer of i.MX7ULP, there is a certain
> probability causes it to reset. The reason is that the CMD32EN of the
> WDOG Timer of i.MX7ULP is disabled in bootloader. The unlock sequence
> are two 16-bit writes to the CNT register within 16 bus clocks. Adding
> mb() is to guarantee that two 16-bit writes are finished within 16 bus
> clocks. Memory barriers cannot be added between these two 16-bit writes
> so that writel_relaxed is used.
> 
> Suggested-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Ye Li <ye.li@nxp.com>

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

> ---
> 
> Changes for v2:
>  - add the reason why memory barriers are added for unlock sequence in commit log
> 
>  drivers/watchdog/imx7ulp_wdt.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index 014f497ea0dc..b8ac0cb04d2f 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -179,9 +179,13 @@ static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
>  	int ret;
>  
>  	local_irq_disable();
> +
> +	mb();
>  	/* unlock the wdog for reconfiguration */
>  	writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
>  	writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> +	mb();
> +
>  	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
>  	if (ret)
>  		goto init_out;
> -- 
> 2.17.1
> 

Re: [PATCH v2 3/7] watchdog: imx7ulp_wdt: Check CMD32EN in wdog init

[Guenter Roeck]

On Thu, Aug 25, 2022 at 04:32:52PM +0800, Alice Guo (OSS) wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> When bootloader has enabled the CMD32EN bit, switch to use 32bits
> unlock command to unlock the CS register. Using 32bits command will
> help on avoiding 16 bus cycle window violation for two 16 bits
> commands.
> 
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Acked-by: Jason Liu <jason.hui.liu@nxp.com>

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

> ---
> 
> Changes for v2:
>  - none
> 
>  drivers/watchdog/imx7ulp_wdt.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index b8ac0cb04d2f..a0f6b8cea78f 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -180,11 +180,16 @@ static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
>  
>  	local_irq_disable();
>  
> -	mb();
> -	/* unlock the wdog for reconfiguration */
> -	writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
> -	writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> -	mb();
> +	val = readl(base + WDOG_CS);
> +	if (val & WDOG_CS_CMD32EN) {
> +		writel(UNLOCK, base + WDOG_CNT);
> +	} else {
> +		mb();
> +		/* unlock the wdog for reconfiguration */
> +		writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
> +		writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> +		mb();
> +	}
>  
>  	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
>  	if (ret)
> -- 
> 2.17.1
> 

Re: [PATCH v2 4/7] watchdog: imx7ulp_wdt: Fix RCS timeout issue

[Guenter Roeck]

On Thu, Aug 25, 2022 at 04:32:53PM +0800, Alice Guo (OSS) wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> According to measure on i.MX7ULP and i.MX8ULP, the RCS done needs
> about 3400us and 6700us respectively. So current 20us timeout is
> not enough. When reconfiguring is on-going, unlock and configure CS
> will lead to unknown result.
> 
> Increase the wait timeout value to 10ms and check the return value
> of RCS wait to fix the issue
> 
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Acked-by: Jason Liu <jason.hui.liu@nxp.com>

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

> ---
> 
> Changes for v2:
>  - none
> 
>  drivers/watchdog/imx7ulp_wdt.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index a0f6b8cea78f..12715c248688 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -39,7 +39,7 @@
>  #define DEFAULT_TIMEOUT	60
>  #define MAX_TIMEOUT	128
>  #define WDOG_CLOCK_RATE	1000
> -#define WDOG_WAIT_TIMEOUT	20
> +#define WDOG_WAIT_TIMEOUT	10000
>  
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0000);
> @@ -80,7 +80,7 @@ static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
>  		writel(val | WDOG_CS_EN, wdt->base + WDOG_CS);
>  	else
>  		writel(val & ~WDOG_CS_EN, wdt->base + WDOG_CS);
> -	imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
> +	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
>  
>  enable_out:
>  	local_irq_enable();
> @@ -127,7 +127,9 @@ static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
>  	if (ret)
>  		goto timeout_out;
>  	writel(val, wdt->base + WDOG_TOVAL);
> -	imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
> +	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
> +	if (ret)
> +		goto timeout_out;
>  
>  	wdog->timeout = timeout;
>  
> -- 
> 2.17.1
> 

Re: [PATCH v2 5/7] watchdog: imx7ulp_wdt: Handle wdog reconfigure failure

[Guenter Roeck]

On Thu, Aug 25, 2022 at 04:32:54PM +0800, Alice Guo (OSS) wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> Current driver may meet reconfigure failure caused by below reasons:
> 
> 1. The wdog on iMX7ULP has different behavior after RCS valid. It needs
>    to wait more than 2.5 wdog clock for clock sync before next
>    reconfiguration, while imx8ulp wdog does not need such delay.
> 
> 2. After unlock, there is 128 bus clock window opened for reconfiguration,
>    but on iMX8ULP, the HW can't guarantee the latency. So it is possible
>    the window is closed before the writing arrives to wdog.
> 
> 3. If the PRES is enabled, the RCS valid time becomes x256 to the time
>    of PRES disabled. It is about 1715ms on iMX8ULP. So We have to increase
>    the RCS timeout and can't wait it in IRQ disabled.
> 
> The patch updates the driver to handle failures
> 
> 1. Using different wait for unlock and RCS. Unlock valid time is very short
>    and only related to bus clock. It must be in IRQ disabled to avoid
>    being interrupted in 128 clock window. But for RCS time, it is longer
>    and ok for IRQ enabled.
> 
> 2. Add retry for any reconfigure failure with default 5 times.
> 
> 3. Add "fsl,imx8ulp-wdt" compatile string for iMX8ULP and afterwards
>    platform which don't need more 2.5 wdog clock after RCS valid.
>    For imx7ulp, add post delay of 2.5 clock after RCS valid.
> 
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>

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

> ---
> 
> Changes for v2:
>  - the wait timeout value of the RCS is 10ms, so use the sleep_us of
>    readl_poll_timeout in imx7ulp_wdt_wait_rcs to avoid a 10ms hot wait
> 
>  drivers/watchdog/imx7ulp_wdt.c | 163 ++++++++++++++++++++++++++-------
>  1 file changed, 129 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index 12715c248688..0cafa86fff7f 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -14,7 +14,9 @@
>  #include <linux/watchdog.h>
>  
>  #define WDOG_CS			0x0
> +#define WDOG_CS_FLG		BIT(14)
>  #define WDOG_CS_CMD32EN		BIT(13)
> +#define WDOG_CS_PRES		BIT(12)
>  #define WDOG_CS_ULK		BIT(11)
>  #define WDOG_CS_RCS		BIT(10)
>  #define LPO_CLK			0x1
> @@ -39,7 +41,11 @@
>  #define DEFAULT_TIMEOUT	60
>  #define MAX_TIMEOUT	128
>  #define WDOG_CLOCK_RATE	1000
> -#define WDOG_WAIT_TIMEOUT	10000
> +#define WDOG_ULK_WAIT_TIMEOUT	1000
> +#define WDOG_RCS_WAIT_TIMEOUT	10000
> +#define WDOG_RCS_POST_WAIT 3000
> +
> +#define RETRY_MAX 5
>  
>  static bool nowayout = WATCHDOG_NOWAYOUT;
>  module_param(nowayout, bool, 0000);
> @@ -50,40 +56,82 @@ struct imx7ulp_wdt_device {
>  	struct watchdog_device wdd;
>  	void __iomem *base;
>  	struct clk *clk;
> +	bool post_rcs_wait;
>  };
>  
> -static int imx7ulp_wdt_wait(void __iomem *base, u32 mask)
> +static int imx7ulp_wdt_wait_ulk(void __iomem *base)
>  {
>  	u32 val = readl(base + WDOG_CS);
>  
> -	if (!(val & mask) && readl_poll_timeout_atomic(base + WDOG_CS, val,
> -						       val & mask, 0,
> -						       WDOG_WAIT_TIMEOUT))
> +	if (!(val & WDOG_CS_ULK) &&
> +	    readl_poll_timeout_atomic(base + WDOG_CS, val,
> +				      val & WDOG_CS_ULK, 0,
> +				      WDOG_ULK_WAIT_TIMEOUT))
>  		return -ETIMEDOUT;
>  
>  	return 0;
>  }
>  
> -static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
> +static int imx7ulp_wdt_wait_rcs(struct imx7ulp_wdt_device *wdt)
>  {
> -	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +	int ret = 0;
> +	u32 val = readl(wdt->base + WDOG_CS);
> +	u64 timeout = (val & WDOG_CS_PRES) ?
> +		WDOG_RCS_WAIT_TIMEOUT * 256 : WDOG_RCS_WAIT_TIMEOUT;
> +	unsigned long wait_min = (val & WDOG_CS_PRES) ?
> +		WDOG_RCS_POST_WAIT * 256 : WDOG_RCS_POST_WAIT;
>  
> +	if (!(val & WDOG_CS_RCS) &&
> +	    readl_poll_timeout(wdt->base + WDOG_CS, val, val & WDOG_CS_RCS, 100,
> +			       timeout))
> +		ret = -ETIMEDOUT;
> +
> +	/* Wait 2.5 clocks after RCS done */
> +	if (wdt->post_rcs_wait)
> +		usleep_range(wait_min, wait_min + 2000);
> +
> +	return ret;
> +}
> +
> +static int _imx7ulp_wdt_enable(struct imx7ulp_wdt_device *wdt, bool enable)
> +{
>  	u32 val = readl(wdt->base + WDOG_CS);
>  	int ret;
>  
>  	local_irq_disable();
>  	writel(UNLOCK, wdt->base + WDOG_CNT);
> -	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_ULK);
> +	ret = imx7ulp_wdt_wait_ulk(wdt->base);
>  	if (ret)
>  		goto enable_out;
>  	if (enable)
>  		writel(val | WDOG_CS_EN, wdt->base + WDOG_CS);
>  	else
>  		writel(val & ~WDOG_CS_EN, wdt->base + WDOG_CS);
> -	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
> +
> +	local_irq_enable();
> +	ret = imx7ulp_wdt_wait_rcs(wdt);
> +
> +	return ret;
>  
>  enable_out:
>  	local_irq_enable();
> +	return ret;
> +}
> +
> +static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
> +{
> +	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +	int ret;
> +	u32 val;
> +	u32 loop = RETRY_MAX;
> +
> +	do {
> +		ret = _imx7ulp_wdt_enable(wdt, enable);
> +		val = readl(wdt->base + WDOG_CS);
> +	} while (--loop > 0 && ((!!(val & WDOG_CS_EN)) != enable || ret));
> +
> +	if (loop == 0)
> +		return -EBUSY;
>  
>  	return ret;
>  }
> @@ -114,28 +162,44 @@ static int imx7ulp_wdt_stop(struct watchdog_device *wdog)
>  	return imx7ulp_wdt_enable(wdog, false);
>  }
>  
> -static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
> -				   unsigned int timeout)
> +static int _imx7ulp_wdt_set_timeout(struct imx7ulp_wdt_device *wdt,
> +				   unsigned int toval)
>  {
> -	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> -	u32 val = WDOG_CLOCK_RATE * timeout;
>  	int ret;
>  
>  	local_irq_disable();
>  	writel(UNLOCK, wdt->base + WDOG_CNT);
> -	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_ULK);
> +	ret = imx7ulp_wdt_wait_ulk(wdt->base);
>  	if (ret)
>  		goto timeout_out;
> -	writel(val, wdt->base + WDOG_TOVAL);
> -	ret = imx7ulp_wdt_wait(wdt->base, WDOG_CS_RCS);
> -	if (ret)
> -		goto timeout_out;
> -
> -	wdog->timeout = timeout;
> +	writel(toval, wdt->base + WDOG_TOVAL);
> +	local_irq_enable();
> +	ret = imx7ulp_wdt_wait_rcs(wdt);
> +	return ret;
>  
>  timeout_out:
>  	local_irq_enable();
> +	return ret;
> +}
>  
> +static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
> +				   unsigned int timeout)
> +{
> +	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +	u32 toval = WDOG_CLOCK_RATE * timeout;
> +	u32 val;
> +	int ret;
> +	u32 loop = RETRY_MAX;
> +
> +	do {
> +		ret = _imx7ulp_wdt_set_timeout(wdt, toval);
> +		val = readl(wdt->base + WDOG_TOVAL);
> +	} while (--loop > 0 && (val != toval || ret));
> +
> +	if (loop == 0)
> +		return -EBUSY;
> +
> +	wdog->timeout = timeout;
>  	return ret;
>  }
>  
> @@ -175,38 +239,59 @@ static const struct watchdog_info imx7ulp_wdt_info = {
>  		    WDIOF_MAGICCLOSE,
>  };
>  
> -static int imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
> +static int _imx7ulp_wdt_init(struct imx7ulp_wdt_device *wdt, unsigned int timeout, unsigned int cs)
>  {
>  	u32 val;
>  	int ret;
>  
>  	local_irq_disable();
>  
> -	val = readl(base + WDOG_CS);
> +	val = readl(wdt->base + WDOG_CS);
>  	if (val & WDOG_CS_CMD32EN) {
> -		writel(UNLOCK, base + WDOG_CNT);
> +		writel(UNLOCK, wdt->base + WDOG_CNT);
>  	} else {
>  		mb();
>  		/* unlock the wdog for reconfiguration */
> -		writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
> -		writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> +		writel_relaxed(UNLOCK_SEQ0, wdt->base + WDOG_CNT);
> +		writel_relaxed(UNLOCK_SEQ1, wdt->base + WDOG_CNT);
>  		mb();
>  	}
>  
> -	ret = imx7ulp_wdt_wait(base, WDOG_CS_ULK);
> +	ret = imx7ulp_wdt_wait_ulk(wdt->base);
>  	if (ret)
>  		goto init_out;
>  
>  	/* set an initial timeout value in TOVAL */
> -	writel(timeout, base + WDOG_TOVAL);
> -	/* enable 32bit command sequence and reconfigure */
> -	val = WDOG_CS_CMD32EN | WDOG_CS_CLK | WDOG_CS_UPDATE |
> -	      WDOG_CS_WAIT | WDOG_CS_STOP;
> -	writel(val, base + WDOG_CS);
> -	imx7ulp_wdt_wait(base, WDOG_CS_RCS);
> +	writel(timeout, wdt->base + WDOG_TOVAL);
> +	writel(cs, wdt->base + WDOG_CS);
> +	local_irq_enable();
> +	ret = imx7ulp_wdt_wait_rcs(wdt);
> +
> +	return ret;
>  
>  init_out:
>  	local_irq_enable();
> +	return ret;
> +}
> +
> +static int imx7ulp_wdt_init(struct imx7ulp_wdt_device *wdt, unsigned int timeout)
> +{
> +	/* enable 32bit command sequence and reconfigure */
> +	u32 val = WDOG_CS_CMD32EN | WDOG_CS_CLK | WDOG_CS_UPDATE |
> +		  WDOG_CS_WAIT | WDOG_CS_STOP;
> +	u32 cs, toval;
> +	int ret;
> +	u32 loop = RETRY_MAX;
> +
> +	do {
> +		ret = _imx7ulp_wdt_init(wdt, timeout, val);
> +		toval = readl(wdt->base + WDOG_TOVAL);
> +		cs = readl(wdt->base + WDOG_CS);
> +		cs &= ~(WDOG_CS_FLG | WDOG_CS_ULK | WDOG_CS_RCS);
> +	} while (--loop > 0 && (cs != val || toval != timeout || ret));
> +
> +	if (loop == 0)
> +		return -EBUSY;
>  
>  	return ret;
>  }
> @@ -239,6 +324,15 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx7ulp_wdt->clk);
>  	}
>  
> +	imx7ulp_wdt->post_rcs_wait = true;
> +	if (of_device_is_compatible(dev->of_node,
> +				    "fsl,imx8ulp-wdt")) {
> +		dev_info(dev, "imx8ulp wdt probe\n");
> +		imx7ulp_wdt->post_rcs_wait = false;
> +	} else {
> +		dev_info(dev, "imx7ulp wdt probe\n");
> +	}
> +
>  	ret = clk_prepare_enable(imx7ulp_wdt->clk);
>  	if (ret)
>  		return ret;
> @@ -259,7 +353,7 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
>  	watchdog_stop_on_reboot(wdog);
>  	watchdog_stop_on_unregister(wdog);
>  	watchdog_set_drvdata(wdog, imx7ulp_wdt);
> -	ret = imx7ulp_wdt_init(imx7ulp_wdt->base, wdog->timeout * WDOG_CLOCK_RATE);
> +	ret = imx7ulp_wdt_init(imx7ulp_wdt, wdog->timeout * WDOG_CLOCK_RATE);
>  	if (ret)
>  		return ret;
>  
> @@ -289,7 +383,7 @@ static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
>  		return ret;
>  
>  	if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
> -		imx7ulp_wdt_init(imx7ulp_wdt->base, timeout);
> +		imx7ulp_wdt_init(imx7ulp_wdt, timeout);
>  
>  	if (watchdog_active(&imx7ulp_wdt->wdd))
>  		imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
> @@ -303,6 +397,7 @@ static const struct dev_pm_ops imx7ulp_wdt_pm_ops = {
>  };
>  
>  static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
> +	{ .compatible = "fsl,imx8ulp-wdt", },
>  	{ .compatible = "fsl,imx7ulp-wdt", },
>  	{ /* sentinel */ }
>  };
> -- 
> 2.17.1
> 

Re: [PATCH v2 6/7] watchdog: imx7ulp_wdt: init wdog when it was active

[Guenter Roeck]

On Thu, Aug 25, 2022 at 04:32:55PM +0800, Alice Guo (OSS) wrote:
> From: Jason Liu <jason.hui.liu@nxp.com>
> 
> Paired with suspend, we can only init wdog again when it was active
> and ping it once to avoid the watchdog timeout after it resumed.
> 
> Signed-off-by: Jason Liu <jason.hui.liu@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>
> Reviewed-by: Ye Li <ye.li@nxp.com>
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Tested-by: Jacky Bai <ping.bai@nxp.com>

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

> ---
> 
> Changes for v2:
>  - none
> 
>  drivers/watchdog/imx7ulp_wdt.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index 0cafa86fff7f..dee02c2a52c9 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -136,13 +136,6 @@ static int imx7ulp_wdt_enable(struct watchdog_device *wdog, bool enable)
>  	return ret;
>  }
>  
> -static bool imx7ulp_wdt_is_enabled(void __iomem *base)
> -{
> -	u32 val = readl(base + WDOG_CS);
> -
> -	return val & WDOG_CS_EN;
> -}
> -
>  static int imx7ulp_wdt_ping(struct watchdog_device *wdog)
>  {
>  	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> @@ -382,11 +375,11 @@ static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
> +	if (watchdog_active(&imx7ulp_wdt->wdd)) {
>  		imx7ulp_wdt_init(imx7ulp_wdt, timeout);
> -
> -	if (watchdog_active(&imx7ulp_wdt->wdd))
>  		imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
> +		imx7ulp_wdt_ping(&imx7ulp_wdt->wdd);
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.17.1
> 

Re: [PATCH v2 7/7] watchdog: imx93: add watchdog timer on imx93

[Guenter Roeck]

On Thu, Aug 25, 2022 at 04:32:56PM +0800, Alice Guo (OSS) wrote:
> From: Alice Guo <alice.guo@nxp.com>
> 
> The WDOG clocks are sourced from lpo_clk, and lpo_clk is the fixed
> 32KHz. TOVAL contains the 16-bit value used to set the timeout period of
> the watchdog. When the timeout period exceeds 2 seconds, the value
> written to the TOVAL register is larger than 16-bit can represent.
> Enabling watchdog prescaler can solve this problem.
> 
> Two points need to be aware of:
> 1. watchdog prescaler enables a fixed 256 pre-scaling of watchdog
> counter reference clock
> 2. reconfiguration takes about 55ms on imx93
> 
> Reviewed-by: Jacky Bai <ping.bai@nxp.com>
> Signed-off-by: Alice Guo <alice.guo@nxp.com>

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

> ---
> 
> Changes for v2:
>  - none
> 
>  drivers/watchdog/imx7ulp_wdt.c | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index dee02c2a52c9..2897902090b3 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -9,6 +9,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/reboot.h>
>  #include <linux/watchdog.h>
> @@ -52,11 +53,17 @@ module_param(nowayout, bool, 0000);
>  MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
>  		 __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
>  
> +struct imx_wdt_hw_feature {
> +	bool prescaler_enable;
> +	u32 wdog_clock_rate;
> +};
> +
>  struct imx7ulp_wdt_device {
>  	struct watchdog_device wdd;
>  	void __iomem *base;
>  	struct clk *clk;
>  	bool post_rcs_wait;
> +	const struct imx_wdt_hw_feature *hw;
>  };
>  
>  static int imx7ulp_wdt_wait_ulk(void __iomem *base)
> @@ -179,7 +186,7 @@ static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
>  				   unsigned int timeout)
>  {
>  	struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> -	u32 toval = WDOG_CLOCK_RATE * timeout;
> +	u32 toval = wdt->hw->wdog_clock_rate * timeout;
>  	u32 val;
>  	int ret;
>  	u32 loop = RETRY_MAX;
> @@ -276,6 +283,9 @@ static int imx7ulp_wdt_init(struct imx7ulp_wdt_device *wdt, unsigned int timeout
>  	int ret;
>  	u32 loop = RETRY_MAX;
>  
> +	if (wdt->hw->prescaler_enable)
> +		val |= WDOG_CS_PRES;
> +
>  	do {
>  		ret = _imx7ulp_wdt_init(wdt, timeout, val);
>  		toval = readl(wdt->base + WDOG_TOVAL);
> @@ -346,7 +356,9 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev)
>  	watchdog_stop_on_reboot(wdog);
>  	watchdog_stop_on_unregister(wdog);
>  	watchdog_set_drvdata(wdog, imx7ulp_wdt);
> -	ret = imx7ulp_wdt_init(imx7ulp_wdt, wdog->timeout * WDOG_CLOCK_RATE);
> +
> +	imx7ulp_wdt->hw = of_device_get_match_data(dev);
> +	ret = imx7ulp_wdt_init(imx7ulp_wdt, wdog->timeout * imx7ulp_wdt->hw->wdog_clock_rate);
>  	if (ret)
>  		return ret;
>  
> @@ -368,7 +380,7 @@ static int __maybe_unused imx7ulp_wdt_suspend_noirq(struct device *dev)
>  static int __maybe_unused imx7ulp_wdt_resume_noirq(struct device *dev)
>  {
>  	struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
> -	u32 timeout = imx7ulp_wdt->wdd.timeout * WDOG_CLOCK_RATE;
> +	u32 timeout = imx7ulp_wdt->wdd.timeout * imx7ulp_wdt->hw->wdog_clock_rate;
>  	int ret;
>  
>  	ret = clk_prepare_enable(imx7ulp_wdt->clk);
> @@ -389,9 +401,20 @@ static const struct dev_pm_ops imx7ulp_wdt_pm_ops = {
>  				      imx7ulp_wdt_resume_noirq)
>  };
>  
> +static const struct imx_wdt_hw_feature imx7ulp_wdt_hw = {
> +	.prescaler_enable = false,
> +	.wdog_clock_rate = 1000,
> +};
> +
> +static const struct imx_wdt_hw_feature imx93_wdt_hw = {
> +	.prescaler_enable = true,
> +	.wdog_clock_rate = 125,
> +};
> +
>  static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
> -	{ .compatible = "fsl,imx8ulp-wdt", },
> -	{ .compatible = "fsl,imx7ulp-wdt", },
> +	{ .compatible = "fsl,imx8ulp-wdt", .data = &imx7ulp_wdt_hw, },
> +	{ .compatible = "fsl,imx7ulp-wdt", .data = &imx7ulp_wdt_hw, },
> +	{ .compatible = "fsl,imx93-wdt", .data = &imx93_wdt_hw, },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, imx7ulp_wdt_dt_ids);
> -- 
> 2.17.1
>