Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1234817 > unrolled thread

[PATCH v4 0/9] watchdog: Add support for keepalives triggered by infrastructure

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/9] watchdog: Add support for keepalives triggered by infrastructure Guenter Roeck <linux@roeck-us.net> - 2015-09-29 10:30 +0200
    [PATCH v4 9/9] watchdog: Add support for minimum time between heartbeats Guenter Roeck <linux@roeck-us.net> - 2015-09-29 10:30 +0200
    [PATCH v4 7/9] watchdog: retu: Convert to use infrastructure triggered keepalives Guenter Roeck <linux@roeck-us.net> - 2015-09-29 10:40 +0200

#1234817 — [PATCH v4 0/9] watchdog: Add support for keepalives triggered by infrastructure

The watchdog infrastructure is currently purely passive, meaning
it only passes information from user space to drivers and vice versa.

Since watchdog hardware tends to have its own quirks, this can result
in quite complex watchdog drivers. A number of scanarios are especially common.

- A watchdog is always active and can not be disabled, or can not be disabled
  once enabled. To support such hardware, watchdog drivers have to implement
  their own timers and use those timers to trigger watchdog keepalives while
  the watchdog device is not or not yet opened.
- A variant of this is the desire to enable a watchdog as soon as its driver
  has been instantiated, to protect the system while it is still booting up,
  but the watchdog daemon is not yet running.
- Some watchdogs have a very short maximum timeout, in the range of just a few
  seconds. Such low timeouts are difficult if not impossible to support from
  user space. Drivers supporting such watchdog hardware need to implement
  a timer function to augment heartbeats from user space.

This patch set solves the above problems while keeping changes to the
watchdog core minimal.

- A new status flag, WDOG_RUNNING, informs the watchdog subsystem that a
  watchdog is running, and that the watchdog subsystem needs to generate
  heartbeat requests while the associated watchdog device is closed.
- A new parameter in the watchdog data structure, max_hw_timeout_ms, informs
  the watchdog subsystem about a maximum hardware timeout. The watchdog
  subsystem uses this information together with the configured timeout
  and the maximum permitted timeout to determine if it needs to generate
  additional heartbeat requests.

As part of this patchset, the semantics of the 'timeout' variable and of
the WDOG_ACTIVE flag are changed slightly.

Per the current watchdog kernel API, the 'timeout' variable is supposed
to reflect the actual hardware watcdog timeout. WDOG_ACTIVE is supposed
to reflect if the hardware watchdog is running or not.

Unfortunately, this does not always reflect reality. In drivers which solve
the above mentioned problems internally, 'timeout' is the watchdog timeout
as seen from user space, and WDOG_ACTIVE reflects that user space is expected
to send keepalive requests to the watchdog driver.

After this patch set is applied, this so far inofficial interpretation
is the 'official' semantics for the timeout variable and the WDOG_ACTIVE
flag. In other words, both values no longer reflect the hardware watchdog
status, but its status as seen from user space.

Patch #1 and #2 are preparatory patches.

Patch #3 adds timer functionality to the watchdog core. It solves the problem
of short maximum hardware timeouts by augmenting heartbeats triggered from
user space with internally triggered heartbeats.

Patch #4 adds functionality to generate heartbeats while the watchdog device is
closed. It handles situation where where the watchdog is running after
the driver has been instantiated, but the device is not yet opened,
and post-close situations necessary if a watchdog can not be stopped.

Patch #5 makes the set_timeout function optional. This is now possible since
timeout changes can now be completely handled in the watchdog core, for
example if the hardware watchdog timeout is fixed.

Patch #6 to #8 are example conversions of some watchdog drivers.
Those patches will require testing.

Patch #9 adds code to unconditionally ensure that the minimum timeout meets
constraints provided by the watchdog driver.

The patch set is also available in branch watchdog-timer of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git.

This patch set does not solve all limitations of the watchdog subsystem.
Specifically, it does not add support for the following features.

- It is desirable to be able to specify a maximum early timeout,
  from booting the system to opening the watchdog device.

This and other features will be addressed with subsequent patches.

It may be possible to simplify the watchdog_update_worker() function
to not require the 'cancel' argument. This is left for further study.

The patch set is inspired by an earlier patch set from Timo Kokonnen.

v4:
- Rebased to v4.3-rc3
- Rearranged patch sequence
- Dropped gpio driver patch. The driver was changed since v4.2,
  and merging the changes turned out to be too difficult.
- Various other cleanups as listed in individual patches
v3:
- Rebased to v4.2-rc8
- Reworked and cleaned up some of the functions.
- No longer call the worker update function if all that is needed is to stop
  the worker.
- max_timeout will now be ignored if max_hw_timeout_ms is provided.
- Added patch 9/9.
v2:
- Rebased to v4.2-rc5
- Improved and hopefully clarified documentation.
- Rearranged variables in struct watchdog_device such that internal variables
  come last.
- The code now ensures that the watchdog times out <timeout> seconds after
  the most recent keepalive sent from user space.
- The internal keepalive now stops silently and no longer generates a
  warning message. Reason is that it will now stop early, while there
  may still be a substantial amount of time for keepalives from user space
  to arrive. If such keepalives arrive late (for example if user space
  is configured to send keepalives just a few seconds before the watchdog
  times out), the message would just be noise and not provide any value.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1234818 — [PATCH v4 9/9] watchdog: Add support for minimum time between heartbeats

Some watchdogs require a minimum time between heartbeats.
Examples are the watchdogs in DA9062 and AT91SAM9x.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 Documentation/watchdog/watchdog-kernel-api.txt |  4 ++++
 drivers/watchdog/watchdog_dev.c                | 12 ++++++++++++
 include/linux/watchdog.h                       |  6 ++++++
 3 files changed, 22 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index f480a9355b43..449b8f12873a 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -53,11 +53,13 @@ struct watchdog_device {
 	unsigned int timeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
+	unsigned int min_hw_heartbeat_ms;
 	unsigned int max_hw_timeout_ms;
 	void *driver_data;
 	unsigned long status;
 	struct mutex lock;
 	unsigned long last_keepalive;
+	unsigned long last_hw_keepalive;
 	struct delayed_work work;
 	struct list_head deferred;
 };
@@ -83,6 +85,8 @@ It contains following fields:
 * max_timeout: the watchdog timer's maximum timeout value (in seconds),
   as seen from userspace. If set, the maximum configurable value for
   'timeout'. Not used if max_hw_timeout_ms is non-zero.
+* min_hw_heartbeat_ms: Minimum time between heartbeats send to the chip,
+  in milli-seconds.
 * max_hw_timeout_ms: Maximum hardware timeout, in milli-seconds.
   If set, the infrastructure will send heartbeats to the watchdog driver
   if 'timeout' is larger than max_hw_timeout, unless WDOG_ACTIVE
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index b590611b176a..568a14462e17 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -111,6 +111,8 @@ static inline void watchdog_update_worker(struct watchdog_device *wdd,
 
 static int _watchdog_ping(struct watchdog_device *wdd)
 {
+	unsigned long earliest_keepalive = wdd->last_hw_keepalive +
+				msecs_to_jiffies(wdd->min_hw_heartbeat_ms);
 	int err;
 
 	if (test_bit(WDOG_UNREGISTERED, &wdd->status))
@@ -119,6 +121,13 @@ static int _watchdog_ping(struct watchdog_device *wdd)
 	if (!watchdog_active(wdd) && !watchdog_running(wdd))
 		return 0;
 
+	if (time_is_after_jiffies(earliest_keepalive)) {
+		mod_delayed_work(watchdog_wq, &wdd->work,
+				 earliest_keepalive - jiffies);
+		return 0;
+	}
+
+	wdd->last_hw_keepalive = jiffies;
 	if (wdd->ops->ping)
 		err = wdd->ops->ping(wdd);  /* ping the watchdog */
 	else
@@ -661,6 +670,9 @@ int watchdog_dev_register(struct watchdog_device *wdd)
 		return err;
 	}
 
+	/* Record time of most recent heartbeat as 'just before now'. */
+	wdd->last_hw_keepalive = jiffies - 1;
+
 	/*
 	 * If the watchdog is running, prevent its driver from being unloaded,
 	 * and schedule an immediate ping.
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 15d04e0fa926..5f83f63ab2cc 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -66,6 +66,8 @@ struct watchdog_ops {
  * @max_timeout:The watchdog devices maximum timeout value, in seconds,
  *		as configurable from user space. Only relevant if
  *		max_hw_timeout_ms is not provided.
+ * @min_hw_heartbeat_ms:
+ *		Minimum time between heartbeats, in milli-seconds.
  * @max_hw_timeout_ms:
  *		Hardware limit for maximum timeout, in milli-seconds.
  *		Replaces max_timeout if specified.
@@ -75,6 +77,8 @@ struct watchdog_ops {
  * @last_keepalive:
  *		Time of most recent keepalive triggered from user space,
  *		in jiffies (watchdog core internal).
+ * @last_hw_keepalive:
+ *		Time of most recent keepalive sent to the driver, in jiffies.
  * @work:	Data structure for worker function (watchdog core internal).
  * @deferred:	entry in wtd_deferred_reg_list which is used to
  *		register early initialized watchdogs.
@@ -99,6 +103,7 @@ struct watchdog_device {
 	unsigned int timeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
+	unsigned int min_hw_heartbeat_ms;
 	unsigned int max_hw_timeout_ms;
 	void *driver_data;
 	unsigned long status;
@@ -112,6 +117,7 @@ struct watchdog_device {
 	/* the following variables are for internal use only */
 	struct mutex lock;
 	unsigned long last_keepalive;
+	unsigned long last_hw_keepalive;
 	struct delayed_work work;
 	struct list_head deferred;
 };
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1234819 — [PATCH v4 7/9] watchdog: retu: Convert to use infrastructure triggered keepalives

The watchdog infrastructure now supports handling watchdog keepalive
if the watchdog is running while the watchdog device is closed.
Convert the driver to use this infrastructure.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: No changes
v2: No changes
---
 drivers/watchdog/retu_wdt.c | 78 ++++-----------------------------------------
 1 file changed, 7 insertions(+), 71 deletions(-)

diff --git a/drivers/watchdog/retu_wdt.c b/drivers/watchdog/retu_wdt.c
index 39cd51df2ffc..9a85c687824c 100644
--- a/drivers/watchdog/retu_wdt.c
+++ b/drivers/watchdog/retu_wdt.c
@@ -28,69 +28,22 @@
 /* Watchdog timer values in seconds */
 #define RETU_WDT_MAX_TIMER	63
 
-struct retu_wdt_dev {
-	struct retu_dev		*rdev;
-	struct device		*dev;
-	struct delayed_work	ping_work;
-};
-
-/*
- * Since Retu watchdog cannot be disabled in hardware, we must kick it
- * with a timer until userspace watchdog software takes over. If
- * CONFIG_WATCHDOG_NOWAYOUT is set, we never start the feeding.
- */
-static void retu_wdt_ping_enable(struct retu_wdt_dev *wdev)
-{
-	retu_write(wdev->rdev, RETU_REG_WATCHDOG, RETU_WDT_MAX_TIMER);
-	schedule_delayed_work(&wdev->ping_work,
-			round_jiffies_relative(RETU_WDT_MAX_TIMER * HZ / 2));
-}
-
-static void retu_wdt_ping_disable(struct retu_wdt_dev *wdev)
-{
-	retu_write(wdev->rdev, RETU_REG_WATCHDOG, RETU_WDT_MAX_TIMER);
-	cancel_delayed_work_sync(&wdev->ping_work);
-}
-
-static void retu_wdt_ping_work(struct work_struct *work)
-{
-	struct retu_wdt_dev *wdev = container_of(to_delayed_work(work),
-						struct retu_wdt_dev, ping_work);
-	retu_wdt_ping_enable(wdev);
-}
-
 static int retu_wdt_start(struct watchdog_device *wdog)
 {
-	struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
+	struct retu_dev *rdev = watchdog_get_drvdata(wdog);
 
-	retu_wdt_ping_disable(wdev);
+	set_bit(WDOG_RUNNING, &wdog->status);
 
-	return retu_write(wdev->rdev, RETU_REG_WATCHDOG, wdog->timeout);
-}
-
-static int retu_wdt_stop(struct watchdog_device *wdog)
-{
-	struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
-
-	retu_wdt_ping_enable(wdev);
-
-	return 0;
-}
-
-static int retu_wdt_ping(struct watchdog_device *wdog)
-{
-	struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
-
-	return retu_write(wdev->rdev, RETU_REG_WATCHDOG, wdog->timeout);
+	return retu_write(rdev, RETU_REG_WATCHDOG, wdog->timeout);
 }
 
 static int retu_wdt_set_timeout(struct watchdog_device *wdog,
 				unsigned int timeout)
 {
-	struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
+	struct retu_dev *rdev = watchdog_get_drvdata(wdog);
 
 	wdog->timeout = timeout;
-	return retu_write(wdev->rdev, RETU_REG_WATCHDOG, wdog->timeout);
+	return retu_write(rdev, RETU_REG_WATCHDOG, wdog->timeout);
 }
 
 static const struct watchdog_info retu_wdt_info = {
@@ -101,8 +54,6 @@ static const struct watchdog_info retu_wdt_info = {
 static const struct watchdog_ops retu_wdt_ops = {
 	.owner		= THIS_MODULE,
 	.start		= retu_wdt_start,
-	.stop		= retu_wdt_stop,
-	.ping		= retu_wdt_ping,
 	.set_timeout	= retu_wdt_set_timeout,
 };
 
@@ -111,17 +62,12 @@ static int retu_wdt_probe(struct platform_device *pdev)
 	struct retu_dev *rdev = dev_get_drvdata(pdev->dev.parent);
 	bool nowayout = WATCHDOG_NOWAYOUT;
 	struct watchdog_device *retu_wdt;
-	struct retu_wdt_dev *wdev;
 	int ret;
 
 	retu_wdt = devm_kzalloc(&pdev->dev, sizeof(*retu_wdt), GFP_KERNEL);
 	if (!retu_wdt)
 		return -ENOMEM;
 
-	wdev = devm_kzalloc(&pdev->dev, sizeof(*wdev), GFP_KERNEL);
-	if (!wdev)
-		return -ENOMEM;
-
 	retu_wdt->info		= &retu_wdt_info;
 	retu_wdt->ops		= &retu_wdt_ops;
 	retu_wdt->timeout	= RETU_WDT_MAX_TIMER;
@@ -129,22 +75,14 @@ static int retu_wdt_probe(struct platform_device *pdev)
 	retu_wdt->max_timeout	= RETU_WDT_MAX_TIMER;
 	retu_wdt->parent	= &pdev->dev;
 
-	watchdog_set_drvdata(retu_wdt, wdev);
+	watchdog_set_drvdata(retu_wdt, rdev);
 	watchdog_set_nowayout(retu_wdt, nowayout);
 
-	wdev->rdev		= rdev;
-	wdev->dev		= &pdev->dev;
-
-	INIT_DELAYED_WORK(&wdev->ping_work, retu_wdt_ping_work);
-
 	ret = watchdog_register_device(retu_wdt);
 	if (ret < 0)
 		return ret;
 
-	if (nowayout)
-		retu_wdt_ping(retu_wdt);
-	else
-		retu_wdt_ping_enable(wdev);
+	retu_wdt_start(retu_wdt);
 
 	platform_set_drvdata(pdev, retu_wdt);
 
@@ -154,10 +92,8 @@ static int retu_wdt_probe(struct platform_device *pdev)
 static int retu_wdt_remove(struct platform_device *pdev)
 {
 	struct watchdog_device *wdog = platform_get_drvdata(pdev);
-	struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
 
 	watchdog_unregister_device(wdog);
-	cancel_delayed_work_sync(&wdev->ping_work);
 
 	return 0;
 }
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web