[RFT PATCH] watchdog: gpio: Convert to use infrastructure triggered keepalives

[RFT PATCH] watchdog: gpio: Convert to use infrastructure triggered keepalives

[Guenter Roeck]

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

The watchdog infrastructure now supports handling watchdog keepalive
if the watchdog is running while the watchdog device is closed.
The infrastructure now also supports generating additional heartbeats
if the maximum hardware timeout is smaller than or close to the
configured timeout. Convert the driver to use this infrastructure.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
This patch applies on top of the patch series introducing infrastructure
triggered keepalives. Compile tested only.

 drivers/watchdog/gpio_wdt.c | 73 ++++++++-------------------------------------
 1 file changed, 13 insertions(+), 60 deletions(-)

diff --git a/drivers/watchdog/gpio_wdt.c b/drivers/watchdog/gpio_wdt.c
index ba066e4a707b..0d46ea02c2ef 100644
--- a/drivers/watchdog/gpio_wdt.c
+++ b/drivers/watchdog/gpio_wdt.c
@@ -18,7 +18,6 @@
 
 #define SOFT_TIMEOUT_MIN	1
 #define SOFT_TIMEOUT_DEF	60
-#define SOFT_TIMEOUT_MAX	0xffff
 
 enum {
 	HW_ALGO_TOGGLE,
@@ -30,11 +29,7 @@ struct gpio_wdt_priv {
 	bool			active_low;
 	bool			state;
 	bool			always_running;
-	bool			armed;
 	unsigned int		hw_algo;
-	unsigned int		hw_margin;
-	unsigned long		last_jiffies;
-	struct timer_list	timer;
 	struct watchdog_device	wdd;
 };
 
@@ -47,21 +42,10 @@ static void gpio_wdt_disable(struct gpio_wdt_priv *priv)
 		gpio_direction_input(priv->gpio);
 }
 
-static void gpio_wdt_hwping(unsigned long data)
+static int gpio_wdt_ping(struct watchdog_device *wdd)
 {
-	struct watchdog_device *wdd = (struct watchdog_device *)data;
 	struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);
 
-	if (priv->armed && time_after(jiffies, priv->last_jiffies +
-				      msecs_to_jiffies(wdd->timeout * 1000))) {
-		dev_crit(wdd->parent,
-			 "Timer expired. System will reboot soon!\n");
-		return;
-	}
-
-	/* Restart timer */
-	mod_timer(&priv->timer, jiffies + priv->hw_margin);
-
 	switch (priv->hw_algo) {
 	case HW_ALGO_TOGGLE:
 		/* Toggle output pin */
@@ -75,55 +59,33 @@ static void gpio_wdt_hwping(unsigned long data)
 		gpio_set_value_cansleep(priv->gpio, priv->active_low);
 		break;
 	}
-}
-
-static void gpio_wdt_start_impl(struct gpio_wdt_priv *priv)
-{
-	priv->state = priv->active_low;
-	gpio_direction_output(priv->gpio, priv->state);
-	priv->last_jiffies = jiffies;
-	gpio_wdt_hwping((unsigned long)&priv->wdd);
+	return 0;
 }
 
 static int gpio_wdt_start(struct watchdog_device *wdd)
 {
 	struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);
 
-	gpio_wdt_start_impl(priv);
-	priv->armed = true;
+	priv->state = priv->active_low;
+	gpio_direction_output(priv->gpio, priv->state);
 
-	return 0;
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+
+	return gpio_wdt_ping(wdd);
 }
 
 static int gpio_wdt_stop(struct watchdog_device *wdd)
 {
 	struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);
 
-	priv->armed = false;
 	if (!priv->always_running) {
-		mod_timer(&priv->timer, 0);
 		gpio_wdt_disable(priv);
+		clear_bit(WDOG_HW_RUNNING, &wdd->status);
 	}
 
 	return 0;
 }
 
-static int gpio_wdt_ping(struct watchdog_device *wdd)
-{
-	struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);
-
-	priv->last_jiffies = jiffies;
-
-	return 0;
-}
-
-static int gpio_wdt_set_timeout(struct watchdog_device *wdd, unsigned int t)
-{
-	wdd->timeout = t;
-
-	return gpio_wdt_ping(wdd);
-}
-
 static const struct watchdog_info gpio_wdt_ident = {
 	.options	= WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
 			  WDIOF_SETTIMEOUT,
@@ -135,7 +97,6 @@ static const struct watchdog_ops gpio_wdt_ops = {
 	.start		= gpio_wdt_start,
 	.stop		= gpio_wdt_stop,
 	.ping		= gpio_wdt_ping,
-	.set_timeout	= gpio_wdt_set_timeout,
 };
 
 static int gpio_wdt_probe(struct platform_device *pdev)
@@ -183,9 +144,6 @@ static int gpio_wdt_probe(struct platform_device *pdev)
 	if (hw_margin < 2 || hw_margin > 65535)
 		return -EINVAL;
 
-	/* 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");
 
@@ -194,31 +152,26 @@ static int gpio_wdt_probe(struct platform_device *pdev)
 	priv->wdd.info		= &gpio_wdt_ident;
 	priv->wdd.ops		= &gpio_wdt_ops;
 	priv->wdd.min_timeout	= SOFT_TIMEOUT_MIN;
-	priv->wdd.max_timeout	= SOFT_TIMEOUT_MAX;
+	priv->wdd.max_hw_timeout_ms = hw_margin;
 	priv->wdd.parent	= &pdev->dev;
 
 	if (watchdog_init_timeout(&priv->wdd, 0, &pdev->dev) < 0)
 		priv->wdd.timeout = SOFT_TIMEOUT_DEF;
 
-	setup_timer(&priv->timer, gpio_wdt_hwping, (unsigned long)&priv->wdd);
-
 	watchdog_stop_on_reboot(&priv->wdd);
 
-	ret = watchdog_register_device(&priv->wdd);
-	if (ret)
-		return ret;
-
 	if (priv->always_running)
-		gpio_wdt_start_impl(priv);
+		gpio_wdt_start(&priv->wdd);
 
-	return 0;
+	ret = watchdog_register_device(&priv->wdd);
+
+	return ret;
 }
 
 static int gpio_wdt_remove(struct platform_device *pdev)
 {
 	struct gpio_wdt_priv *priv = platform_get_drvdata(pdev);
 
-	del_timer_sync(&priv->timer);
 	watchdog_unregister_device(&priv->wdd);
 
 	return 0;
-- 
2.1.4

Re: [RFT PATCH] watchdog: gpio: Convert to use infrastructure triggered keepalives

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4495 bytes --]

Hi Guenter,

[auto build test ERROR on v4.5-rc1]
[also build test ERROR on next-20160125]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Guenter-Roeck/watchdog-gpio-Convert-to-use-infrastructure-triggered-keepalives/20160126-231612
config: i386-randconfig-x005-01261041 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/watchdog/gpio_wdt.c: In function 'gpio_wdt_start':
>> drivers/watchdog/gpio_wdt.c:72:10: error: 'WDOG_HW_RUNNING' undeclared (first use in this function)
     set_bit(WDOG_HW_RUNNING, &wdd->status);
             ^
   drivers/watchdog/gpio_wdt.c:72:10: note: each undeclared identifier is reported only once for each function it appears in
   drivers/watchdog/gpio_wdt.c: In function 'gpio_wdt_stop':
   drivers/watchdog/gpio_wdt.c:83:13: error: 'WDOG_HW_RUNNING' undeclared (first use in this function)
      clear_bit(WDOG_HW_RUNNING, &wdd->status);
                ^
   drivers/watchdog/gpio_wdt.c: In function 'gpio_wdt_probe':
>> drivers/watchdog/gpio_wdt.c:155:11: error: 'struct watchdog_device' has no member named 'max_hw_timeout_ms'
     priv->wdd.max_hw_timeout_ms = hw_margin;
              ^

vim +/WDOG_HW_RUNNING +72 drivers/watchdog/gpio_wdt.c

    66	{
    67		struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);
    68	
    69		priv->state = priv->active_low;
    70		gpio_direction_output(priv->gpio, priv->state);
    71	
  > 72		set_bit(WDOG_HW_RUNNING, &wdd->status);
    73	
    74		return gpio_wdt_ping(wdd);
    75	}
    76	
    77	static int gpio_wdt_stop(struct watchdog_device *wdd)
    78	{
    79		struct gpio_wdt_priv *priv = watchdog_get_drvdata(wdd);
    80	
    81		if (!priv->always_running) {
    82			gpio_wdt_disable(priv);
    83			clear_bit(WDOG_HW_RUNNING, &wdd->status);
    84		}
    85	
    86		return 0;
    87	}
    88	
    89	static const struct watchdog_info gpio_wdt_ident = {
    90		.options	= WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING |
    91				  WDIOF_SETTIMEOUT,
    92		.identity	= "GPIO Watchdog",
    93	};
    94	
    95	static const struct watchdog_ops gpio_wdt_ops = {
    96		.owner		= THIS_MODULE,
    97		.start		= gpio_wdt_start,
    98		.stop		= gpio_wdt_stop,
    99		.ping		= gpio_wdt_ping,
   100	};
   101	
   102	static int gpio_wdt_probe(struct platform_device *pdev)
   103	{
   104		struct gpio_wdt_priv *priv;
   105		enum of_gpio_flags flags;
   106		unsigned int hw_margin;
   107		unsigned long f = 0;
   108		const char *algo;
   109		int ret;
   110	
   111		priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
   112		if (!priv)
   113			return -ENOMEM;
   114	
   115		priv->gpio = of_get_gpio_flags(pdev->dev.of_node, 0, &flags);
   116		if (!gpio_is_valid(priv->gpio))
   117			return priv->gpio;
   118	
   119		priv->active_low = flags & OF_GPIO_ACTIVE_LOW;
   120	
   121		ret = of_property_read_string(pdev->dev.of_node, "hw_algo", &algo);
   122		if (ret)
   123			return ret;
   124		if (!strcmp(algo, "toggle")) {
   125			priv->hw_algo = HW_ALGO_TOGGLE;
   126			f = GPIOF_IN;
   127		} else if (!strcmp(algo, "level")) {
   128			priv->hw_algo = HW_ALGO_LEVEL;
   129			f = priv->active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW;
   130		} else {
   131			return -EINVAL;
   132		}
   133	
   134		ret = devm_gpio_request_one(&pdev->dev, priv->gpio, f,
   135					    dev_name(&pdev->dev));
   136		if (ret)
   137			return ret;
   138	
   139		ret = of_property_read_u32(pdev->dev.of_node,
   140					   "hw_margin_ms", &hw_margin);
   141		if (ret)
   142			return ret;
   143		/* Disallow values lower than 2 and higher than 65535 ms */
   144		if (hw_margin < 2 || hw_margin > 65535)
   145			return -EINVAL;
   146	
   147		priv->always_running = of_property_read_bool(pdev->dev.of_node,
   148							     "always-running");
   149	
   150		watchdog_set_drvdata(&priv->wdd, priv);
   151	
   152		priv->wdd.info		= &gpio_wdt_ident;
   153		priv->wdd.ops		= &gpio_wdt_ops;
   154		priv->wdd.min_timeout	= SOFT_TIMEOUT_MIN;
 > 155		priv->wdd.max_hw_timeout_ms = hw_margin;
   156		priv->wdd.parent	= &pdev->dev;
   157	
   158		if (watchdog_init_timeout(&priv->wdd, 0, &pdev->dev) < 0)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23545 bytes --]

Re: [RFT PATCH] watchdog: gpio: Convert to use infrastructure triggered keepalives

[Guenter Roeck]

On 01/26/2016 07:40 AM, kbuild test robot wrote:
> Hi Guenter,
>
> [auto build test ERROR on v4.5-rc1]
> [also build test ERROR on next-20160125]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Guenter-Roeck/watchdog-gpio-Convert-to-use-infrastructure-triggered-keepalives/20160126-231612
> config: i386-randconfig-x005-01261041 (attached as .config)
> reproduce:
>          # save the attached .config to linux build tree
>          make ARCH=i386
>

Depends on [1].

Guenter

---
[1] https://lkml.org/lkml/2016/1/25/1302