Re: [PATCH v2] gpio_wdt: Add "always_running" feature to GPIO watchdog

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/20/2014 10:53 PM, Mike Looijmans wrote:
On some chips, like the TPS386000, the trigger cannot be disabled
and the CPU must keep toggling the line at all times. Add a switch
"always_running" to keep toggling the GPIO line regardless of the
state of the soft part of the watchdog. The "armed" member keeps
track of whether a timeout must also cause a reset.

Signed-off-by: Mike Looijmans <mike.looijmans@xxxxxxxx>
---

v2: Rename property "always_running" to "always-running" and add devicetree
documentation

  .../devicetree/bindings/watchdog/gpio-wdt.txt      |    5 +++++
  drivers/watchdog/gpio_wdt.c                        |   20 +++++++++++++++++---
  2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
index 37afec1..1987949 100644
--- a/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
@@ -13,6 +13,11 @@ Required Properties:
      by the GPIO flags.
  - hw_margin_ms: Maximum time to reset watchdog circuit (milliseconds).

+Optional Properties:
+- always-running: If the watchdog timer cannot be disabled, add this flag to
+  have the driver keep toggling the signal without a client. It will only cease
+  to toggle the signal when the device is open and the timeout elapsed.
+
  Example:
  	watchdog: watchdog {
  		/* ADM706 */
diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
index 220a9e0..3266215 100644
--- a/drivers/watchdog/gpio_wdt.c
+++ b/drivers/watchdog/gpio_wdt.c
@@ -31,6 +31,8 @@ struct gpio_wdt_priv {
  	int			gpio;
  	bool			active_low;
  	bool			state;
+	bool		always_running;
+	bool		armed;

Please align columns with the other variables.

  	unsigned int		hw_algo;
  	unsigned int		hw_margin;
  	unsigned long		last_jiffies;
@@ -56,6 +58,7 @@ static int gpio_wdt_start(struct watchdog_device *wdd)
  	gpio_direction_output(priv->gpio, priv->state);
  	priv->last_jiffies = jiffies;
  	mod_timer(&priv->timer, priv->last_jiffies + priv->hw_margin);
+	priv->armed = true;

  	return 0;
  }
@@ -64,8 +67,11 @@ static int gpio_wdt_stop(struct watchdog_device *wdd)
  {
  	struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);

-	mod_timer(&priv->timer, 0);
-	gpio_wdt_disable(priv);
+	priv->armed = false;
+	if (!priv->always_running) {
+		mod_timer(&priv->timer, 0);
+		gpio_wdt_disable(priv);
+	}

  	return 0;
  }
@@ -91,7 +97,7 @@ static void gpio_wdt_hwping(unsigned long data)
  	struct watchdog_device *wdd = (struct watchdog_device *)data;
  	struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);

-	if (time_after(jiffies, priv->last_jiffies +
+	if (priv->armed && time_after(jiffies, priv->last_jiffies +
  		       msecs_to_jiffies(wdd->timeout * 1000))) {

Please align continuation lines.

  		dev_crit(wdd->dev, "Timer expired. System will reboot soon!\n");
  		return;
@@ -197,6 +203,9 @@ static int gpio_wdt_probe(struct platform_device *pdev)
  	/* Use safe value (1/2 of real timeout) */
  	priv->hw_margin = msecs_to_jiffies(hw_margin / 2);

+	priv->always_running = of_property_read_bool(pdev->dev.of_node,
+				"always-running");
+
Please align continuation line with '('.

  	watchdog_set_drvdata(&priv->wdd, priv);

  	priv->wdd.info		= &gpio_wdt_ident;
@@ -218,6 +227,11 @@ static int gpio_wdt_probe(struct platform_device *pdev)
  	if (ret)
  		watchdog_unregister_device(&priv->wdd);

+	if (priv->always_running) {
+		gpio_wdt_start(&priv->wdd);
+		priv->armed = false;
+	}
+

This won't work if there is an error. The driver won't load but the timer
is started, which will likely cause a crash when the timer fires. You'll
have to fix the error handling.

I also dislike that you first set priv->armed in the start function and
then reset it, but I don't have a better idea right now.

Guenter

  	return ret;
  }



--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux