[PATCH 1/3] watchdog: omap_wdt: Fix WDT timeout configuration

[PATCH 1/3] watchdog: omap_wdt: Fix WDT timeout configuration

[Marek Vasut]

The timeout parameter of omap3_wdt_start() is in miliseconds, while
GET_WLDR_VAL() expects parameter in seconds. Fix this so the WDT
driver is actually usable.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Suniel Mahesh <sunil.m@techveda.org>
---
 drivers/watchdog/omap_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 284cfbb2a8..b9cdf70036 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -188,7 +188,7 @@ static int omap3_wdt_stop(struct udevice *dev)
 static int omap3_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 {
 	struct omap3_wdt_priv *priv = dev_get_priv(dev);
-	u32 pre_margin = GET_WLDR_VAL(timeout_ms);
+	u32 pre_margin = GET_WLDR_VAL(timeout_ms / 1000);
 /*
  * Make sure the watchdog is disabled. This is unfortunately required
  * because writing to various registers with the watchdog running has no
-- 
2.24.1

[PATCH 2/3] watchdog: omap_wdt: Fix WDT reloading

[Marek Vasut]

The watchdog timer value was never updated in the hardware by this
driver, so the watchdog triggered on some random stale value that
was left in the hardware. The TI SPRUH37C says, quote:

  20.4.3.9 Modifying Timer Count/Load Values and Prescaler Setting
  ...
  After a write access, the load register value and prescaler ratio
  registers are updated immediately, but new values are considered
  only after the next consecutive counter overflow or after a new
  trigger command (the WDT_WTGR register).

This means at least one trigger must happen. The driver probably
depended on someone calling it's .reset() callback, however that
is not guaranteed e.g. if the WDT operates without servicing.

Add this missing trigger.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Suniel Mahesh <sunil.m@techveda.org>
---
 drivers/watchdog/omap_wdt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index b9cdf70036..85425ca505 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -219,6 +219,16 @@ static int omap3_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 	while ((readl(&priv->regs->wdtwwps)) & WDT_WWPS_PEND_WSPR)
 		;
 
+	/* Trigger the watchdog to actually reload the counter. */
+	while ((readl(&priv->regs->wdtwwps)) & WDT_WWPS_PEND_WTGR)
+		;
+
+	priv->wdt_trgr_pattern = ~(priv->wdt_trgr_pattern);
+	writel(priv->wdt_trgr_pattern, &priv->regs->wdtwtgr);
+
+	while ((readl(&priv->regs->wdtwwps)) & WDT_WWPS_PEND_WTGR)
+		;
+
 	return 0;
 }
 
-- 
2.24.1

[PATCH 3/3] watchdog: omap_wdt: Fix WDT coding style

[Marek Vasut]

Fix obvious coding style problems, no functional change.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: Sam Protsenko <semen.protsenko@linaro.org>
Cc: Suniel Mahesh <sunil.m@techveda.org>
---
 drivers/watchdog/omap_wdt.c | 44 ++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 85425ca505..5199d914ed 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -150,24 +150,24 @@ static int omap3_wdt_reset(struct udevice *dev)
 {
 	struct omap3_wdt_priv *priv = dev_get_priv(dev);
 
-/*
- * Somebody just triggered watchdog reset and write to WTGR register
- * is in progress. It is resetting right now, no need to trigger it
- * again
- */
+	/*
+	 * Somebody just triggered watchdog reset and write to WTGR register
+	 * is in progress. It is resetting right now, no need to trigger it
+	 * again
+	 */
 	if ((readl(&priv->regs->wdtwwps)) & WDT_WWPS_PEND_WTGR)
 		return 0;
 
 	priv->wdt_trgr_pattern = ~(priv->wdt_trgr_pattern);
 	writel(priv->wdt_trgr_pattern, &priv->regs->wdtwtgr);
-/*
- * Don't wait for posted write to complete, i.e. don't check
- * WDT_WWPS_PEND_WTGR bit in WWPS register. There is no writes to
- * WTGR register outside of this func, and if entering it
- * we see WDT_WWPS_PEND_WTGR bit set, it means watchdog reset
- * was just triggered. This prevents us from wasting time in busy
- * polling of WDT_WWPS_PEND_WTGR bit.
- */
+	/*
+	 * Don't wait for posted write to complete, i.e. don't check
+	 * WDT_WWPS_PEND_WTGR bit in WWPS register. There is no writes to
+	 * WTGR register outside of this func, and if entering it
+	 * we see WDT_WWPS_PEND_WTGR bit set, it means watchdog reset
+	 * was just triggered. This prevents us from wasting time in busy
+	 * polling of WDT_WWPS_PEND_WTGR bit.
+	 */
 	return 0;
 }
 
@@ -175,7 +175,7 @@ static int omap3_wdt_stop(struct udevice *dev)
 {
 	struct omap3_wdt_priv *priv = dev_get_priv(dev);
 
-/* disable watchdog */
+	/* disable watchdog */
 	writel(0xAAAA, &priv->regs->wdtwspr);
 	while (readl(&priv->regs->wdtwwps) != 0x0)
 		;
@@ -189,28 +189,28 @@ static int omap3_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
 {
 	struct omap3_wdt_priv *priv = dev_get_priv(dev);
 	u32 pre_margin = GET_WLDR_VAL(timeout_ms / 1000);
-/*
- * Make sure the watchdog is disabled. This is unfortunately required
- * because writing to various registers with the watchdog running has no
- * effect.
- */
+	/*
+	 * Make sure the watchdog is disabled. This is unfortunately required
+	 * because writing to various registers with the watchdog running has
+	 * no effect.
+	 */
 	omap3_wdt_stop(dev);
 
-/* initialize prescaler */
+	/* initialize prescaler */
 	while (readl(&priv->regs->wdtwwps) & WDT_WWPS_PEND_WCLR)
 		;
 
 	writel(WDT_WCLR_PRE | (PTV << WDT_WCLR_PTV_OFF), &priv->regs->wdtwclr);
 	while (readl(&priv->regs->wdtwwps) & WDT_WWPS_PEND_WCLR)
 		;
-/* just count up at 32 KHz */
+	/* just count up at 32 KHz */
 	while (readl(&priv->regs->wdtwwps) & WDT_WWPS_PEND_WLDR)
 		;
 
 	writel(pre_margin, &priv->regs->wdtwldr);
 	while (readl(&priv->regs->wdtwwps) & WDT_WWPS_PEND_WLDR)
 		;
-/* Sequence to enable the watchdog */
+	/* Sequence to enable the watchdog */
 	writel(0xBBBB, &priv->regs->wdtwspr);
 	while ((readl(&priv->regs->wdtwwps)) & WDT_WWPS_PEND_WSPR)
 		;
-- 
2.24.1

[PATCH 1/3] watchdog: omap_wdt: Fix WDT timeout configuration

[Lokesh Vutla]

On 24/01/20 10:14 AM, Marek Vasut wrote:
> The timeout parameter of omap3_wdt_start() is in miliseconds, while
> GET_WLDR_VAL() expects parameter in seconds. Fix this so the WDT
> driver is actually usable.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Suniel Mahesh <sunil.m@techveda.org>

Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

A cover letter would really be nice to get a brief on the series.

Thanks and regards,
Lokesh

[PATCH 2/3] watchdog: omap_wdt: Fix WDT reloading

[Lokesh Vutla]

On 24/01/20 10:14 AM, Marek Vasut wrote:
> The watchdog timer value was never updated in the hardware by this
> driver, so the watchdog triggered on some random stale value that
> was left in the hardware. The TI SPRUH37C says, quote:
> 
>   20.4.3.9 Modifying Timer Count/Load Values and Prescaler Setting
>   ...
>   After a write access, the load register value and prescaler ratio
>   registers are updated immediately, but new values are considered
>   only after the next consecutive counter overflow or after a new
>   trigger command (the WDT_WTGR register).
> 
> This means at least one trigger must happen. The driver probably
> depended on someone calling it's .reset() callback, however that
> is not guaranteed e.g. if the WDT operates without servicing.
> 
> Add this missing trigger.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Suniel Mahesh <sunil.m@techveda.org>

Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

[PATCH 3/3] watchdog: omap_wdt: Fix WDT coding style

[Lokesh Vutla]

On 24/01/20 10:14 AM, Marek Vasut wrote:
> Fix obvious coding style problems, no functional change.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Suniel Mahesh <sunil.m@techveda.org>
> ---
>  drivers/watchdog/omap_wdt.c | 44 ++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)

Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

[PATCH 1/3] watchdog: omap_wdt: Fix WDT timeout configuration

[Lokesh Vutla]

On 24/01/20 10:14 AM, Marek Vasut wrote:
> The timeout parameter of omap3_wdt_start() is in miliseconds, while
> GET_WLDR_VAL() expects parameter in seconds. Fix this so the WDT
> driver is actually usable.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Cc: Sam Protsenko <semen.protsenko@linaro.org>
> Cc: Suniel Mahesh <sunil.m@techveda.org>

Series pulled into u-boot-ti tree.

Thanks and regards,
Lokesh