[PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

Hi all,
this patchset add the timeout-sec property to the watchdog core,
to the at91sam9_wdt driver and on all framework enabled watchdog driver
with dt support.
The at91sam9_wdt is tested on evk-pr3,
the other drivers are compile tested only.

Best regards
Fabio Porcedda

changes:
v7:
  - rebase over v3.8-rc2
  - remove the patch already merged
v6:
  - remove patch for function of_get_timeout_sec as aked by Rob Herring
  - add Acked-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
  - add Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
v5:
  - fix codestyle for compilance ti checkpatch.pl about
    "ERROR: do not initialise statics to 0 or NULL"
    for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
v4:
  - add of_get_timeout_sec
  - add s3c2410_wdt driver
  - split driver patch
  - fixed watchdog_init_timeout handling of min_timeout and max_timeout
v3:
  - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
  - move the watchdog_init_timeout function from inline to watchdog_core.c
  - add timeout parameter handling code to watchdog_init_timeout
  - add timeout-sec property to the pnx4008-wdt driver
  - add two small commit for fixup and cleanup
v2:
  - change "timeout" to "timeout-sec" as asked by Jean-Christophe
  - at91sam9_wdt: use the new helper function
  - at91sam9_wdt: add bounds checking
  - watchdog.h: add bounds checking

Fabio Porcedda (6):
  watchdog: core: dt: add support for the timeout-sec dt property
  watchdog: orion_wdt: add timeout-sec property binding
  watchdog: pnx4008: add timeout-sec property binding
  watchdog: s3c2410_wdt: add timeout-sec property binding
  watchdog: at91sam9_wdt: add timeout-sec property binding
  watchdog: orion_wdt: move the min_timeout initialization

 .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
 .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
 .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
 .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
 Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
 drivers/watchdog/Kconfig                           |  1 +
 drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
 drivers/watchdog/orion_wdt.c                       | 10 +++---
 drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
 drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
 drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
 include/linux/watchdog.h                           |  3 ++
 12 files changed, 91 insertions(+), 15 deletions(-)

-- 
1.8.0.3

[PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

Hi all,
this patchset add the timeout-sec property to the watchdog core,
to the at91sam9_wdt driver and on all framework enabled watchdog driver
with dt support.
The at91sam9_wdt is tested on evk-pr3,
the other drivers are compile tested only.

Best regards
Fabio Porcedda

changes:
v7:
  - rebase over v3.8-rc2
  - remove the patch already merged
v6:
  - remove patch for function of_get_timeout_sec as aked by Rob Herring
  - add Acked-by: Jason Cooper <jason@lakedaemon.net>
  - add Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
v5:
  - fix codestyle for compilance ti checkpatch.pl about
    "ERROR: do not initialise statics to 0 or NULL"
    for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
v4:
  - add of_get_timeout_sec
  - add s3c2410_wdt driver
  - split driver patch
  - fixed watchdog_init_timeout handling of min_timeout and max_timeout
v3:
  - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
  - move the watchdog_init_timeout function from inline to watchdog_core.c
  - add timeout parameter handling code to watchdog_init_timeout
  - add timeout-sec property to the pnx4008-wdt driver
  - add two small commit for fixup and cleanup
v2:
  - change "timeout" to "timeout-sec" as asked by Jean-Christophe
  - at91sam9_wdt: use the new helper function
  - at91sam9_wdt: add bounds checking
  - watchdog.h: add bounds checking

Fabio Porcedda (6):
  watchdog: core: dt: add support for the timeout-sec dt property
  watchdog: orion_wdt: add timeout-sec property binding
  watchdog: pnx4008: add timeout-sec property binding
  watchdog: s3c2410_wdt: add timeout-sec property binding
  watchdog: at91sam9_wdt: add timeout-sec property binding
  watchdog: orion_wdt: move the min_timeout initialization

 .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
 .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
 .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
 .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
 Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
 drivers/watchdog/Kconfig                           |  1 +
 drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
 drivers/watchdog/orion_wdt.c                       | 10 +++---
 drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
 drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
 drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
 include/linux/watchdog.h                           |  3 ++
 12 files changed, 91 insertions(+), 15 deletions(-)

-- 
1.8.0.3

[PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

Hi all,
this patchset add the timeout-sec property to the watchdog core,
to the at91sam9_wdt driver and on all framework enabled watchdog driver
with dt support.
The at91sam9_wdt is tested on evk-pr3,
the other drivers are compile tested only.

Best regards
Fabio Porcedda

changes:
v7:
  - rebase over v3.8-rc2
  - remove the patch already merged
v6:
  - remove patch for function of_get_timeout_sec as aked by Rob Herring
  - add Acked-by: Jason Cooper <jason@lakedaemon.net>
  - add Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
v5:
  - fix codestyle for compilance ti checkpatch.pl about
    "ERROR: do not initialise statics to 0 or NULL"
    for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
v4:
  - add of_get_timeout_sec
  - add s3c2410_wdt driver
  - split driver patch
  - fixed watchdog_init_timeout handling of min_timeout and max_timeout
v3:
  - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
  - move the watchdog_init_timeout function from inline to watchdog_core.c
  - add timeout parameter handling code to watchdog_init_timeout
  - add timeout-sec property to the pnx4008-wdt driver
  - add two small commit for fixup and cleanup
v2:
  - change "timeout" to "timeout-sec" as asked by Jean-Christophe
  - at91sam9_wdt: use the new helper function
  - at91sam9_wdt: add bounds checking
  - watchdog.h: add bounds checking

Fabio Porcedda (6):
  watchdog: core: dt: add support for the timeout-sec dt property
  watchdog: orion_wdt: add timeout-sec property binding
  watchdog: pnx4008: add timeout-sec property binding
  watchdog: s3c2410_wdt: add timeout-sec property binding
  watchdog: at91sam9_wdt: add timeout-sec property binding
  watchdog: orion_wdt: move the min_timeout initialization

 .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
 .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
 .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
 .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
 Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
 drivers/watchdog/Kconfig                           |  1 +
 drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
 drivers/watchdog/orion_wdt.c                       | 10 +++---
 drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
 drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
 drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
 include/linux/watchdog.h                           |  3 ++
 12 files changed, 91 insertions(+), 15 deletions(-)

-- 
1.8.0.3

[PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
 drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
 include/linux/watchdog.h                       |  3 +++
 3 files changed, 50 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..44098c2 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -212,3 +212,13 @@ driver specific data to and a pointer to the data itself.
 The watchdog_get_drvdata function allows you to retrieve driver specific data.
 The argument of this function is the watchdog device where you want to retrieve
 data from. The function returns the pointer to the driver specific data.
+
+To initialize the timeout field use the following function:
+
+extern void watchdog_init_timeout(struct watchdog_device *wdd,
+                                  unsigned int parm_timeout,
+                                  struct device_node *node);
+
+The watchdog_init_timeout function allows you to initialize the timeout field
+using the module timeout parameter or retrieving the timeout-sec property from
+the device tree.
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 3796434..758a285 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -36,12 +36,49 @@
 #include <linux/init.h>		/* For __init/__exit/... */
 #include <linux/idr.h>		/* For ida_* macros */
 #include <linux/err.h>		/* For IS_ERR macros */
+#include <linux/of.h>		/* For of_get_timeout_sec */
 
 #include "watchdog_core.h"	/* For watchdog_dev_register/... */
 
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
+				      unsigned int t)
+
+{
+	if (wdd->min_timeout < wdd->max_timeout)
+		return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
+	else
+		return (t > 0);
+}
+
+/**
+ * watchdog_init_timeout() - initialize the timeout field
+ * @parm: timeout module parameter, it takes precedence over the
+ *        timeout-sec property.
+ * @node: Retrieve the timeout-sec property only if the parm_timeout
+ *        is out of bounds.
+ */
+void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
+			   struct device_node *node)
+{
+	unsigned int t = 0;
+
+	if (watchdog_is_valid_timeout(wdd, parm)) {
+		wdd->timeout = parm;
+		return;
+	}
+
+	/* try to get the timeout_sec property */
+	if (!node)
+		return;
+	of_property_read_u32(node, "timeout-sec", &t);
+	if (watchdog_is_valid_timeout(wdd, t))
+		wdd->timeout = t;
+}
+EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+
 /**
  * watchdog_register_device() - register a watchdog device
  * @wdd: watchdog device
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 3a9df2f..e40cc2b 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -130,6 +130,9 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 }
 
 /* drivers/watchdog/watchdog_core.c */
+extern void watchdog_init_timeout(struct watchdog_device *wdd,
+				  unsigned int parm_timeout,
+				  struct device_node *node);
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
-- 
1.8.0.3

[PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
 drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
 include/linux/watchdog.h                       |  3 +++
 3 files changed, 50 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..44098c2 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -212,3 +212,13 @@ driver specific data to and a pointer to the data itself.
 The watchdog_get_drvdata function allows you to retrieve driver specific data.
 The argument of this function is the watchdog device where you want to retrieve
 data from. The function returns the pointer to the driver specific data.
+
+To initialize the timeout field use the following function:
+
+extern void watchdog_init_timeout(struct watchdog_device *wdd,
+                                  unsigned int parm_timeout,
+                                  struct device_node *node);
+
+The watchdog_init_timeout function allows you to initialize the timeout field
+using the module timeout parameter or retrieving the timeout-sec property from
+the device tree.
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 3796434..758a285 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -36,12 +36,49 @@
 #include <linux/init.h>		/* For __init/__exit/... */
 #include <linux/idr.h>		/* For ida_* macros */
 #include <linux/err.h>		/* For IS_ERR macros */
+#include <linux/of.h>		/* For of_get_timeout_sec */
 
 #include "watchdog_core.h"	/* For watchdog_dev_register/... */
 
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
+				      unsigned int t)
+
+{
+	if (wdd->min_timeout < wdd->max_timeout)
+		return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
+	else
+		return (t > 0);
+}
+
+/**
+ * watchdog_init_timeout() - initialize the timeout field
+ * @parm: timeout module parameter, it takes precedence over the
+ *        timeout-sec property.
+ * @node: Retrieve the timeout-sec property only if the parm_timeout
+ *        is out of bounds.
+ */
+void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
+			   struct device_node *node)
+{
+	unsigned int t = 0;
+
+	if (watchdog_is_valid_timeout(wdd, parm)) {
+		wdd->timeout = parm;
+		return;
+	}
+
+	/* try to get the timeout_sec property */
+	if (!node)
+		return;
+	of_property_read_u32(node, "timeout-sec", &t);
+	if (watchdog_is_valid_timeout(wdd, t))
+		wdd->timeout = t;
+}
+EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+
 /**
  * watchdog_register_device() - register a watchdog device
  * @wdd: watchdog device
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 3a9df2f..e40cc2b 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -130,6 +130,9 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 }
 
 /* drivers/watchdog/watchdog_core.c */
+extern void watchdog_init_timeout(struct watchdog_device *wdd,
+				  unsigned int parm_timeout,
+				  struct device_node *node);
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
-- 
1.8.0.3

[PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
 drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
 include/linux/watchdog.h                       |  3 +++
 3 files changed, 50 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..44098c2 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -212,3 +212,13 @@ driver specific data to and a pointer to the data itself.
 The watchdog_get_drvdata function allows you to retrieve driver specific data.
 The argument of this function is the watchdog device where you want to retrieve
 data from. The function returns the pointer to the driver specific data.
+
+To initialize the timeout field use the following function:
+
+extern void watchdog_init_timeout(struct watchdog_device *wdd,
+                                  unsigned int parm_timeout,
+                                  struct device_node *node);
+
+The watchdog_init_timeout function allows you to initialize the timeout field
+using the module timeout parameter or retrieving the timeout-sec property from
+the device tree.
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 3796434..758a285 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -36,12 +36,49 @@
 #include <linux/init.h>		/* For __init/__exit/... */
 #include <linux/idr.h>		/* For ida_* macros */
 #include <linux/err.h>		/* For IS_ERR macros */
+#include <linux/of.h>		/* For of_get_timeout_sec */
 
 #include "watchdog_core.h"	/* For watchdog_dev_register/... */
 
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
+				      unsigned int t)
+
+{
+	if (wdd->min_timeout < wdd->max_timeout)
+		return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
+	else
+		return (t > 0);
+}
+
+/**
+ * watchdog_init_timeout() - initialize the timeout field
+ * @parm: timeout module parameter, it takes precedence over the
+ *        timeout-sec property.
+ * @node: Retrieve the timeout-sec property only if the parm_timeout
+ *        is out of bounds.
+ */
+void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
+			   struct device_node *node)
+{
+	unsigned int t = 0;
+
+	if (watchdog_is_valid_timeout(wdd, parm)) {
+		wdd->timeout = parm;
+		return;
+	}
+
+	/* try to get the timeout_sec property */
+	if (!node)
+		return;
+	of_property_read_u32(node, "timeout-sec", &t);
+	if (watchdog_is_valid_timeout(wdd, t))
+		wdd->timeout = t;
+}
+EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+
 /**
  * watchdog_register_device() - register a watchdog device
  * @wdd: watchdog device
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 3a9df2f..e40cc2b 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -130,6 +130,9 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 }
 
 /* drivers/watchdog/watchdog_core.c */
+extern void watchdog_init_timeout(struct watchdog_device *wdd,
+				  unsigned int parm_timeout,
+				  struct device_node *node);
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
-- 
1.8.0.3

[PATCH v7 2/6] watchdog: orion_wdt: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
Acked-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
---
 Documentation/devicetree/bindings/watchdog/marvel.txt | 5 +++++
 drivers/watchdog/orion_wdt.c                          | 8 +++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt
index 0b2503a..5dc8d30 100644
--- a/Documentation/devicetree/bindings/watchdog/marvel.txt
+++ b/Documentation/devicetree/bindings/watchdog/marvel.txt
@@ -5,10 +5,15 @@ Required Properties:
 - Compatibility : "marvell,orion-wdt"
 - reg		: Address of the timer registers
 
+Optional properties:
+
+- timeout-sec	: Contains the watchdog timeout in seconds
+
 Example:
 
 	wdt@20300 {
 		compatible = "marvell,orion-wdt";
 		reg = <0x20300 0x28>;
+		timeout-sec = <10>;
 		status = "okay";
 	};
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 7c18b3b..b7018600 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -164,12 +164,10 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
-	if ((heartbeat < 1) || (heartbeat > wdt_max_duration))
-		heartbeat = wdt_max_duration;
-
-	orion_wdt.timeout = heartbeat;
 	orion_wdt.min_timeout = 1;
+	orion_wdt.timeout = wdt_max_duration;
 	orion_wdt.max_timeout = wdt_max_duration;
+	watchdog_init_timeout(&orion_wdt, heartbeat, pdev->dev.of_node);
 
 	watchdog_set_nowayout(&orion_wdt, nowayout);
 	ret = watchdog_register_device(&orion_wdt);
@@ -179,7 +177,7 @@ static int orion_wdt_probe(struct platform_device *pdev)
 	}
 
 	pr_info("Initial timeout %d sec%s\n",
-		heartbeat, nowayout ? ", nowayout" : "");
+		orion_wdt.timeout, nowayout ? ", nowayout" : "");
 	return 0;
 }
 
-- 
1.8.0.3

[PATCH v7 2/6] watchdog: orion_wdt: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
 Documentation/devicetree/bindings/watchdog/marvel.txt | 5 +++++
 drivers/watchdog/orion_wdt.c                          | 8 +++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt
index 0b2503a..5dc8d30 100644
--- a/Documentation/devicetree/bindings/watchdog/marvel.txt
+++ b/Documentation/devicetree/bindings/watchdog/marvel.txt
@@ -5,10 +5,15 @@ Required Properties:
 - Compatibility : "marvell,orion-wdt"
 - reg		: Address of the timer registers
 
+Optional properties:
+
+- timeout-sec	: Contains the watchdog timeout in seconds
+
 Example:
 
 	wdt@20300 {
 		compatible = "marvell,orion-wdt";
 		reg = <0x20300 0x28>;
+		timeout-sec = <10>;
 		status = "okay";
 	};
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 7c18b3b..b7018600 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -164,12 +164,10 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
-	if ((heartbeat < 1) || (heartbeat > wdt_max_duration))
-		heartbeat = wdt_max_duration;
-
-	orion_wdt.timeout = heartbeat;
 	orion_wdt.min_timeout = 1;
+	orion_wdt.timeout = wdt_max_duration;
 	orion_wdt.max_timeout = wdt_max_duration;
+	watchdog_init_timeout(&orion_wdt, heartbeat, pdev->dev.of_node);
 
 	watchdog_set_nowayout(&orion_wdt, nowayout);
 	ret = watchdog_register_device(&orion_wdt);
@@ -179,7 +177,7 @@ static int orion_wdt_probe(struct platform_device *pdev)
 	}
 
 	pr_info("Initial timeout %d sec%s\n",
-		heartbeat, nowayout ? ", nowayout" : "");
+		orion_wdt.timeout, nowayout ? ", nowayout" : "");
 	return 0;
 }
 
-- 
1.8.0.3

[PATCH v7 2/6] watchdog: orion_wdt: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
 Documentation/devicetree/bindings/watchdog/marvel.txt | 5 +++++
 drivers/watchdog/orion_wdt.c                          | 8 +++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt
index 0b2503a..5dc8d30 100644
--- a/Documentation/devicetree/bindings/watchdog/marvel.txt
+++ b/Documentation/devicetree/bindings/watchdog/marvel.txt
@@ -5,10 +5,15 @@ Required Properties:
 - Compatibility : "marvell,orion-wdt"
 - reg		: Address of the timer registers
 
+Optional properties:
+
+- timeout-sec	: Contains the watchdog timeout in seconds
+
 Example:
 
 	wdt at 20300 {
 		compatible = "marvell,orion-wdt";
 		reg = <0x20300 0x28>;
+		timeout-sec = <10>;
 		status = "okay";
 	};
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 7c18b3b..b7018600 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -164,12 +164,10 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
-	if ((heartbeat < 1) || (heartbeat > wdt_max_duration))
-		heartbeat = wdt_max_duration;
-
-	orion_wdt.timeout = heartbeat;
 	orion_wdt.min_timeout = 1;
+	orion_wdt.timeout = wdt_max_duration;
 	orion_wdt.max_timeout = wdt_max_duration;
+	watchdog_init_timeout(&orion_wdt, heartbeat, pdev->dev.of_node);
 
 	watchdog_set_nowayout(&orion_wdt, nowayout);
 	ret = watchdog_register_device(&orion_wdt);
@@ -179,7 +177,7 @@ static int orion_wdt_probe(struct platform_device *pdev)
 	}
 
 	pr_info("Initial timeout %d sec%s\n",
-		heartbeat, nowayout ? ", nowayout" : "");
+		orion_wdt.timeout, nowayout ? ", nowayout" : "");
 	return 0;
 }
 
-- 
1.8.0.3

[PATCH v7 3/6] watchdog: pnx4008: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
Cc: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Masanari Iida <standby24x7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt | 4 ++++
 drivers/watchdog/pnx4008_wdt.c                             | 7 +++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt b/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
index 7c7f688..556d06c 100644
--- a/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
@@ -5,9 +5,13 @@ Required properties:
 - reg: physical base address of the controller and length of memory mapped
   region.
 
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
 Example:
 
 	watchdog@4003C000 {
 		compatible = "nxp,pnx4008-wdt";
 		reg = <0x4003C000 0x1000>;
+		timeout-sec = <10>;
 	};
diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
index dcba5da..aa4ec26 100644
--- a/drivers/watchdog/pnx4008_wdt.c
+++ b/drivers/watchdog/pnx4008_wdt.c
@@ -142,6 +142,7 @@ static const struct watchdog_ops pnx4008_wdt_ops = {
 static struct watchdog_device pnx4008_wdd = {
 	.info = &pnx4008_wdt_ident,
 	.ops = &pnx4008_wdt_ops,
+	.timeout = DEFAULT_HEARTBEAT,
 	.min_timeout = 1,
 	.max_timeout = MAX_HEARTBEAT,
 };
@@ -151,8 +152,7 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret = 0;
 
-	if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
-		heartbeat = DEFAULT_HEARTBEAT;
+	watchdog_init_timeout(&pnx4008_wdd, heartbeat, pdev->dev.of_node);
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	wdt_base = devm_request_and_ioremap(&pdev->dev, r);
@@ -167,7 +167,6 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto out;
 
-	pnx4008_wdd.timeout = heartbeat;
 	pnx4008_wdd.bootstatus = (readl(WDTIM_RES(wdt_base)) & WDOG_RESET) ?
 			WDIOF_CARDRESET : 0;
 	watchdog_set_nowayout(&pnx4008_wdd, nowayout);
@@ -181,7 +180,7 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	}
 
 	dev_info(&pdev->dev, "PNX4008 Watchdog Timer: heartbeat %d sec\n",
-			heartbeat);
+		 pnx4008_wdd.timeout);
 
 	return 0;
 
-- 
1.8.0.3

[PATCH v7 3/6] watchdog: pnx4008: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Masanari Iida <standby24x7@gmail.com>
---
 Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt | 4 ++++
 drivers/watchdog/pnx4008_wdt.c                             | 7 +++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt b/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
index 7c7f688..556d06c 100644
--- a/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
@@ -5,9 +5,13 @@ Required properties:
 - reg: physical base address of the controller and length of memory mapped
   region.
 
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
 Example:
 
 	watchdog@4003C000 {
 		compatible = "nxp,pnx4008-wdt";
 		reg = <0x4003C000 0x1000>;
+		timeout-sec = <10>;
 	};
diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
index dcba5da..aa4ec26 100644
--- a/drivers/watchdog/pnx4008_wdt.c
+++ b/drivers/watchdog/pnx4008_wdt.c
@@ -142,6 +142,7 @@ static const struct watchdog_ops pnx4008_wdt_ops = {
 static struct watchdog_device pnx4008_wdd = {
 	.info = &pnx4008_wdt_ident,
 	.ops = &pnx4008_wdt_ops,
+	.timeout = DEFAULT_HEARTBEAT,
 	.min_timeout = 1,
 	.max_timeout = MAX_HEARTBEAT,
 };
@@ -151,8 +152,7 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret = 0;
 
-	if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
-		heartbeat = DEFAULT_HEARTBEAT;
+	watchdog_init_timeout(&pnx4008_wdd, heartbeat, pdev->dev.of_node);
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	wdt_base = devm_request_and_ioremap(&pdev->dev, r);
@@ -167,7 +167,6 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto out;
 
-	pnx4008_wdd.timeout = heartbeat;
 	pnx4008_wdd.bootstatus = (readl(WDTIM_RES(wdt_base)) & WDOG_RESET) ?
 			WDIOF_CARDRESET : 0;
 	watchdog_set_nowayout(&pnx4008_wdd, nowayout);
@@ -181,7 +180,7 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	}
 
 	dev_info(&pdev->dev, "PNX4008 Watchdog Timer: heartbeat %d sec\n",
-			heartbeat);
+		 pnx4008_wdd.timeout);
 
 	return 0;
 
-- 
1.8.0.3

[PATCH v7 3/6] watchdog: pnx4008: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Wim Van Sebroeck <wim@iguana.be>
Cc: Wolfram Sang <w.sang@pengutronix.de>
Cc: Masanari Iida <standby24x7@gmail.com>
---
 Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt | 4 ++++
 drivers/watchdog/pnx4008_wdt.c                             | 7 +++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt b/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
index 7c7f688..556d06c 100644
--- a/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
@@ -5,9 +5,13 @@ Required properties:
 - reg: physical base address of the controller and length of memory mapped
   region.
 
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
 Example:
 
 	watchdog at 4003C000 {
 		compatible = "nxp,pnx4008-wdt";
 		reg = <0x4003C000 0x1000>;
+		timeout-sec = <10>;
 	};
diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
index dcba5da..aa4ec26 100644
--- a/drivers/watchdog/pnx4008_wdt.c
+++ b/drivers/watchdog/pnx4008_wdt.c
@@ -142,6 +142,7 @@ static const struct watchdog_ops pnx4008_wdt_ops = {
 static struct watchdog_device pnx4008_wdd = {
 	.info = &pnx4008_wdt_ident,
 	.ops = &pnx4008_wdt_ops,
+	.timeout = DEFAULT_HEARTBEAT,
 	.min_timeout = 1,
 	.max_timeout = MAX_HEARTBEAT,
 };
@@ -151,8 +152,7 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret = 0;
 
-	if (heartbeat < 1 || heartbeat > MAX_HEARTBEAT)
-		heartbeat = DEFAULT_HEARTBEAT;
+	watchdog_init_timeout(&pnx4008_wdd, heartbeat, pdev->dev.of_node);
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	wdt_base = devm_request_and_ioremap(&pdev->dev, r);
@@ -167,7 +167,6 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto out;
 
-	pnx4008_wdd.timeout = heartbeat;
 	pnx4008_wdd.bootstatus = (readl(WDTIM_RES(wdt_base)) & WDOG_RESET) ?
 			WDIOF_CARDRESET : 0;
 	watchdog_set_nowayout(&pnx4008_wdd, nowayout);
@@ -181,7 +180,7 @@ static int pnx4008_wdt_probe(struct platform_device *pdev)
 	}
 
 	dev_info(&pdev->dev, "PNX4008 Watchdog Timer: heartbeat %d sec\n",
-			heartbeat);
+		 pnx4008_wdd.timeout);
 
 	return 0;
 
-- 
1.8.0.3

[PATCH v7 4/6] watchdog: s3c2410_wdt: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Ben Dooks <ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
Cc: Kukjin Kim <kgene.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/watchdog/samsung-wdt.txt | 3 +++
 drivers/watchdog/s3c2410_wdt.c                             | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 79ead82..451127a 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -9,3 +9,6 @@ Required properties:
 - reg : base physical address of the controller and length of memory mapped
 	region.
 - interrupts : interrupt number to the cpu.
+
+Optional properties:
+- timeout-sec : contains the watchdog timeout in seconds.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 27bcd4e..2f187ae 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -53,7 +53,7 @@
 #define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME	(15)
 
 static bool nowayout	= WATCHDOG_NOWAYOUT;
-static int tmr_margin	= CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME;
+static int tmr_margin;
 static int tmr_atboot	= CONFIG_S3C2410_WATCHDOG_ATBOOT;
 static int soft_noboot;
 static int debug;
@@ -226,6 +226,7 @@ static struct watchdog_ops s3c2410wdt_ops = {
 static struct watchdog_device s3c2410_wdd = {
 	.info = &s3c2410_wdt_ident,
 	.ops = &s3c2410wdt_ops,
+	.timeout = CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME,
 };
 
 /* interrupt handler code */
@@ -365,7 +366,8 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 	/* see if we can actually set the requested timer margin, and if
 	 * not, try the default value */
 
-	if (s3c2410wdt_set_heartbeat(&s3c2410_wdd, tmr_margin)) {
+	watchdog_init_timeout(&s3c2410_wdd, tmr_margin,  pdev->dev.of_node);
+	if (s3c2410wdt_set_heartbeat(&s3c2410_wdd, s3c2410_wdd.timeout)) {
 		started = s3c2410wdt_set_heartbeat(&s3c2410_wdd,
 					CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
 
-- 
1.8.0.3

[PATCH v7 4/6] watchdog: s3c2410_wdt: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 Documentation/devicetree/bindings/watchdog/samsung-wdt.txt | 3 +++
 drivers/watchdog/s3c2410_wdt.c                             | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 79ead82..451127a 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -9,3 +9,6 @@ Required properties:
 - reg : base physical address of the controller and length of memory mapped
 	region.
 - interrupts : interrupt number to the cpu.
+
+Optional properties:
+- timeout-sec : contains the watchdog timeout in seconds.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 27bcd4e..2f187ae 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -53,7 +53,7 @@
 #define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME	(15)
 
 static bool nowayout	= WATCHDOG_NOWAYOUT;
-static int tmr_margin	= CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME;
+static int tmr_margin;
 static int tmr_atboot	= CONFIG_S3C2410_WATCHDOG_ATBOOT;
 static int soft_noboot;
 static int debug;
@@ -226,6 +226,7 @@ static struct watchdog_ops s3c2410wdt_ops = {
 static struct watchdog_device s3c2410_wdd = {
 	.info = &s3c2410_wdt_ident,
 	.ops = &s3c2410wdt_ops,
+	.timeout = CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME,
 };
 
 /* interrupt handler code */
@@ -365,7 +366,8 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 	/* see if we can actually set the requested timer margin, and if
 	 * not, try the default value */
 
-	if (s3c2410wdt_set_heartbeat(&s3c2410_wdd, tmr_margin)) {
+	watchdog_init_timeout(&s3c2410_wdd, tmr_margin,  pdev->dev.of_node);
+	if (s3c2410wdt_set_heartbeat(&s3c2410_wdd, s3c2410_wdd.timeout)) {
 		started = s3c2410wdt_set_heartbeat(&s3c2410_wdd,
 					CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
 
-- 
1.8.0.3

[PATCH v7 4/6] watchdog: s3c2410_wdt: add timeout-sec property binding

[Fabio Porcedda]

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 Documentation/devicetree/bindings/watchdog/samsung-wdt.txt | 3 +++
 drivers/watchdog/s3c2410_wdt.c                             | 6 ++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
index 79ead82..451127a 100644
--- a/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/samsung-wdt.txt
@@ -9,3 +9,6 @@ Required properties:
 - reg : base physical address of the controller and length of memory mapped
 	region.
 - interrupts : interrupt number to the cpu.
+
+Optional properties:
+- timeout-sec : contains the watchdog timeout in seconds.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 27bcd4e..2f187ae 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -53,7 +53,7 @@
 #define CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME	(15)
 
 static bool nowayout	= WATCHDOG_NOWAYOUT;
-static int tmr_margin	= CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME;
+static int tmr_margin;
 static int tmr_atboot	= CONFIG_S3C2410_WATCHDOG_ATBOOT;
 static int soft_noboot;
 static int debug;
@@ -226,6 +226,7 @@ static struct watchdog_ops s3c2410wdt_ops = {
 static struct watchdog_device s3c2410_wdd = {
 	.info = &s3c2410_wdt_ident,
 	.ops = &s3c2410wdt_ops,
+	.timeout = CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME,
 };
 
 /* interrupt handler code */
@@ -365,7 +366,8 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 	/* see if we can actually set the requested timer margin, and if
 	 * not, try the default value */
 
-	if (s3c2410wdt_set_heartbeat(&s3c2410_wdd, tmr_margin)) {
+	watchdog_init_timeout(&s3c2410_wdd, tmr_margin,  pdev->dev.of_node);
+	if (s3c2410wdt_set_heartbeat(&s3c2410_wdd, s3c2410_wdd.timeout)) {
 		started = s3c2410wdt_set_heartbeat(&s3c2410_wdd,
 					CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
 
-- 
1.8.0.3

[PATCH v7 5/6] watchdog: at91sam9_wdt: add timeout-sec property binding

[Fabio Porcedda]

Tested on at91sam9260 board (evk-pro3).

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Andrew Victor <linux-PelNFVqkFnVyf+4FbqDuWQ@public.gmane.org>
Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
---
 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt |  4 ++++
 drivers/watchdog/Kconfig                                 |  1 +
 drivers/watchdog/at91sam9_wdt.c                          | 16 +++++++++++++---
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
index 2957ebb..fcdd48f 100644
--- a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
@@ -7,9 +7,13 @@ Required properties:
 - reg: physical base address of the controller and length of memory mapped
   region.
 
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
 Example:
 
 	watchdog@fffffd40 {
 		compatible = "atmel,at91sam9260-wdt";
 		reg = <0xfffffd40 0x10>;
+		timeout-sec = <10>;
 	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7f809fd..23cd386 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -124,6 +124,7 @@ config AT91RM9200_WATCHDOG
 config AT91SAM9X_WATCHDOG
 	tristate "AT91SAM9X / AT91CAP9 watchdog"
 	depends on ARCH_AT91 && !ARCH_AT91RM9200
+	select WATCHDOG_CORE
 	help
 	  Watchdog timer embedded into AT91SAM9X and AT91CAP9 chips. This will
 	  reboot your system when the timeout is reached.
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index dc42e44..d864dc4 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -57,8 +57,10 @@
 #define WDT_TIMEOUT	(HZ/2)
 
 /* User land timeout */
+#define MIN_HEARTBEAT 1
+#define MAX_HEARTBEAT 16
 #define WDT_HEARTBEAT 15
-static int heartbeat = WDT_HEARTBEAT;
+static int heartbeat;
 module_param(heartbeat, int, 0);
 MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
 	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
@@ -255,6 +257,12 @@ static struct miscdevice at91wdt_miscdev = {
 	.fops		= &at91wdt_fops,
 };
 
+static struct watchdog_device at91wdt_wdd __initdata = {
+	.timeout = WDT_HEARTBEAT,
+	.min_timeout = MIN_HEARTBEAT,
+	.max_timeout = MAX_HEARTBEAT,
+};
+
 static int __init at91wdt_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
@@ -273,6 +281,8 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	watchdog_init_timeout(&at91wdt_wdd, heartbeat, pdev->dev.of_node);
+
 	/* Set watchdog */
 	res = at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
 	if (res)
@@ -282,12 +292,12 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 	if (res)
 		return res;
 
-	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
+	at91wdt_private.next_heartbeat = jiffies + at91wdt_wdd.timeout * HZ;
 	setup_timer(&at91wdt_private.timer, at91_ping, 0);
 	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
 
 	pr_info("enabled (heartbeat=%d sec, nowayout=%d)\n",
-		heartbeat, nowayout);
+		at91wdt_wdd.timeout, nowayout);
 
 	return 0;
 }
-- 
1.8.0.3

[PATCH v7 5/6] watchdog: at91sam9_wdt: add timeout-sec property binding

[Fabio Porcedda]

Tested on at91sam9260 board (evk-pro3).

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Andrew Victor <linux@maxim.org.za>
Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt |  4 ++++
 drivers/watchdog/Kconfig                                 |  1 +
 drivers/watchdog/at91sam9_wdt.c                          | 16 +++++++++++++---
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
index 2957ebb..fcdd48f 100644
--- a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
@@ -7,9 +7,13 @@ Required properties:
 - reg: physical base address of the controller and length of memory mapped
   region.
 
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
 Example:
 
 	watchdog@fffffd40 {
 		compatible = "atmel,at91sam9260-wdt";
 		reg = <0xfffffd40 0x10>;
+		timeout-sec = <10>;
 	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7f809fd..23cd386 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -124,6 +124,7 @@ config AT91RM9200_WATCHDOG
 config AT91SAM9X_WATCHDOG
 	tristate "AT91SAM9X / AT91CAP9 watchdog"
 	depends on ARCH_AT91 && !ARCH_AT91RM9200
+	select WATCHDOG_CORE
 	help
 	  Watchdog timer embedded into AT91SAM9X and AT91CAP9 chips. This will
 	  reboot your system when the timeout is reached.
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index dc42e44..d864dc4 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -57,8 +57,10 @@
 #define WDT_TIMEOUT	(HZ/2)
 
 /* User land timeout */
+#define MIN_HEARTBEAT 1
+#define MAX_HEARTBEAT 16
 #define WDT_HEARTBEAT 15
-static int heartbeat = WDT_HEARTBEAT;
+static int heartbeat;
 module_param(heartbeat, int, 0);
 MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
 	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
@@ -255,6 +257,12 @@ static struct miscdevice at91wdt_miscdev = {
 	.fops		= &at91wdt_fops,
 };
 
+static struct watchdog_device at91wdt_wdd __initdata = {
+	.timeout = WDT_HEARTBEAT,
+	.min_timeout = MIN_HEARTBEAT,
+	.max_timeout = MAX_HEARTBEAT,
+};
+
 static int __init at91wdt_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
@@ -273,6 +281,8 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	watchdog_init_timeout(&at91wdt_wdd, heartbeat, pdev->dev.of_node);
+
 	/* Set watchdog */
 	res = at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
 	if (res)
@@ -282,12 +292,12 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 	if (res)
 		return res;
 
-	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
+	at91wdt_private.next_heartbeat = jiffies + at91wdt_wdd.timeout * HZ;
 	setup_timer(&at91wdt_private.timer, at91_ping, 0);
 	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
 
 	pr_info("enabled (heartbeat=%d sec, nowayout=%d)\n",
-		heartbeat, nowayout);
+		at91wdt_wdd.timeout, nowayout);
 
 	return 0;
 }
-- 
1.8.0.3

[PATCH v7 5/6] watchdog: at91sam9_wdt: add timeout-sec property binding

[Fabio Porcedda]

Tested on at91sam9260 board (evk-pro3).

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Andrew Victor <linux@maxim.org.za>
Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt |  4 ++++
 drivers/watchdog/Kconfig                                 |  1 +
 drivers/watchdog/at91sam9_wdt.c                          | 16 +++++++++++++---
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
index 2957ebb..fcdd48f 100644
--- a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
@@ -7,9 +7,13 @@ Required properties:
 - reg: physical base address of the controller and length of memory mapped
   region.
 
+Optional properties:
+- timeout-sec: contains the watchdog timeout in seconds.
+
 Example:
 
 	watchdog at fffffd40 {
 		compatible = "atmel,at91sam9260-wdt";
 		reg = <0xfffffd40 0x10>;
+		timeout-sec = <10>;
 	};
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 7f809fd..23cd386 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -124,6 +124,7 @@ config AT91RM9200_WATCHDOG
 config AT91SAM9X_WATCHDOG
 	tristate "AT91SAM9X / AT91CAP9 watchdog"
 	depends on ARCH_AT91 && !ARCH_AT91RM9200
+	select WATCHDOG_CORE
 	help
 	  Watchdog timer embedded into AT91SAM9X and AT91CAP9 chips. This will
 	  reboot your system when the timeout is reached.
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index dc42e44..d864dc4 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -57,8 +57,10 @@
 #define WDT_TIMEOUT	(HZ/2)
 
 /* User land timeout */
+#define MIN_HEARTBEAT 1
+#define MAX_HEARTBEAT 16
 #define WDT_HEARTBEAT 15
-static int heartbeat = WDT_HEARTBEAT;
+static int heartbeat;
 module_param(heartbeat, int, 0);
 MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
 	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
@@ -255,6 +257,12 @@ static struct miscdevice at91wdt_miscdev = {
 	.fops		= &at91wdt_fops,
 };
 
+static struct watchdog_device at91wdt_wdd __initdata = {
+	.timeout = WDT_HEARTBEAT,
+	.min_timeout = MIN_HEARTBEAT,
+	.max_timeout = MAX_HEARTBEAT,
+};
+
 static int __init at91wdt_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
@@ -273,6 +281,8 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	watchdog_init_timeout(&at91wdt_wdd, heartbeat, pdev->dev.of_node);
+
 	/* Set watchdog */
 	res = at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
 	if (res)
@@ -282,12 +292,12 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 	if (res)
 		return res;
 
-	at91wdt_private.next_heartbeat = jiffies + heartbeat * HZ;
+	at91wdt_private.next_heartbeat = jiffies + at91wdt_wdd.timeout * HZ;
 	setup_timer(&at91wdt_private.timer, at91_ping, 0);
 	mod_timer(&at91wdt_private.timer, jiffies + WDT_TIMEOUT);
 
 	pr_info("enabled (heartbeat=%d sec, nowayout=%d)\n",
-		heartbeat, nowayout);
+		at91wdt_wdd.timeout, nowayout);
 
 	return 0;
 }
-- 
1.8.0.3

[PATCH v7 6/6] watchdog: orion_wdt: move the min_timeout initialization

[Fabio Porcedda]

Move the min_timeout initialization inside the orion_wdt definition.

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
Acked-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
---
 drivers/watchdog/orion_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index b7018600..ed05360 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -140,6 +140,7 @@ static const struct watchdog_ops orion_wdt_ops = {
 static struct watchdog_device orion_wdt = {
 	.info = &orion_wdt_info,
 	.ops = &orion_wdt_ops,
+	.min_timeout = 1,
 };
 
 static int orion_wdt_probe(struct platform_device *pdev)
@@ -164,7 +165,6 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
-	orion_wdt.min_timeout = 1;
 	orion_wdt.timeout = wdt_max_duration;
 	orion_wdt.max_timeout = wdt_max_duration;
 	watchdog_init_timeout(&orion_wdt, heartbeat, pdev->dev.of_node);
-- 
1.8.0.3

[PATCH v7 6/6] watchdog: orion_wdt: move the min_timeout initialization

[Fabio Porcedda]

Move the min_timeout initialization inside the orion_wdt definition.

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
 drivers/watchdog/orion_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index b7018600..ed05360 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -140,6 +140,7 @@ static const struct watchdog_ops orion_wdt_ops = {
 static struct watchdog_device orion_wdt = {
 	.info = &orion_wdt_info,
 	.ops = &orion_wdt_ops,
+	.min_timeout = 1,
 };
 
 static int orion_wdt_probe(struct platform_device *pdev)
@@ -164,7 +165,6 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
-	orion_wdt.min_timeout = 1;
 	orion_wdt.timeout = wdt_max_duration;
 	orion_wdt.max_timeout = wdt_max_duration;
 	watchdog_init_timeout(&orion_wdt, heartbeat, pdev->dev.of_node);
-- 
1.8.0.3

[PATCH v7 6/6] watchdog: orion_wdt: move the min_timeout initialization

[Fabio Porcedda]

Move the min_timeout initialization inside the orion_wdt definition.

Only compile tested.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
 drivers/watchdog/orion_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index b7018600..ed05360 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -140,6 +140,7 @@ static const struct watchdog_ops orion_wdt_ops = {
 static struct watchdog_device orion_wdt = {
 	.info = &orion_wdt_info,
 	.ops = &orion_wdt_ops,
+	.min_timeout = 1,
 };
 
 static int orion_wdt_probe(struct platform_device *pdev)
@@ -164,7 +165,6 @@ static int orion_wdt_probe(struct platform_device *pdev)
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
-	orion_wdt.min_timeout = 1;
 	orion_wdt.timeout = wdt_max_duration;
 	orion_wdt.max_timeout = wdt_max_duration;
 	watchdog_init_timeout(&orion_wdt, heartbeat, pdev->dev.of_node);
-- 
1.8.0.3

Re: [PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:04 Tue 08 Jan     , Fabio Porcedda wrote:
> Hi all,
> this patchset add the timeout-sec property to the watchdog core,
> to the at91sam9_wdt driver and on all framework enabled watchdog driver
> with dt support.
> The at91sam9_wdt is tested on evk-pr3,
> the other drivers are compile tested only.
> 

I'll prefer a macro to abstract the string in every driver

except this fine by me

Best Regards,
J.

> Best regards
> Fabio Porcedda
> 
> changes:
> v7:
>   - rebase over v3.8-rc2
>   - remove the patch already merged
> v6:
>   - remove patch for function of_get_timeout_sec as aked by Rob Herring
>   - add Acked-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
>   - add Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
> v5:
>   - fix codestyle for compilance ti checkpatch.pl about
>     "ERROR: do not initialise statics to 0 or NULL"
>     for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
> v4:
>   - add of_get_timeout_sec
>   - add s3c2410_wdt driver
>   - split driver patch
>   - fixed watchdog_init_timeout handling of min_timeout and max_timeout
> v3:
>   - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
>   - move the watchdog_init_timeout function from inline to watchdog_core.c
>   - add timeout parameter handling code to watchdog_init_timeout
>   - add timeout-sec property to the pnx4008-wdt driver
>   - add two small commit for fixup and cleanup
> v2:
>   - change "timeout" to "timeout-sec" as asked by Jean-Christophe
>   - at91sam9_wdt: use the new helper function
>   - at91sam9_wdt: add bounds checking
>   - watchdog.h: add bounds checking
> 
> Fabio Porcedda (6):
>   watchdog: core: dt: add support for the timeout-sec dt property
>   watchdog: orion_wdt: add timeout-sec property binding
>   watchdog: pnx4008: add timeout-sec property binding
>   watchdog: s3c2410_wdt: add timeout-sec property binding
>   watchdog: at91sam9_wdt: add timeout-sec property binding
>   watchdog: orion_wdt: move the min_timeout initialization
> 
>  .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
>  .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
>  .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
>  .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
>  Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
>  drivers/watchdog/Kconfig                           |  1 +
>  drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
>  drivers/watchdog/orion_wdt.c                       | 10 +++---
>  drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
>  drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
>  drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
>  include/linux/watchdog.h                           |  3 ++
>  12 files changed, 91 insertions(+), 15 deletions(-)
> 
> -- 
> 1.8.0.3
> 

Re: [PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:04 Tue 08 Jan     , Fabio Porcedda wrote:
> Hi all,
> this patchset add the timeout-sec property to the watchdog core,
> to the at91sam9_wdt driver and on all framework enabled watchdog driver
> with dt support.
> The at91sam9_wdt is tested on evk-pr3,
> the other drivers are compile tested only.
> 

I'll prefer a macro to abstract the string in every driver

except this fine by me

Best Regards,
J.

> Best regards
> Fabio Porcedda
> 
> changes:
> v7:
>   - rebase over v3.8-rc2
>   - remove the patch already merged
> v6:
>   - remove patch for function of_get_timeout_sec as aked by Rob Herring
>   - add Acked-by: Jason Cooper <jason@lakedaemon.net>
>   - add Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> v5:
>   - fix codestyle for compilance ti checkpatch.pl about
>     "ERROR: do not initialise statics to 0 or NULL"
>     for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
> v4:
>   - add of_get_timeout_sec
>   - add s3c2410_wdt driver
>   - split driver patch
>   - fixed watchdog_init_timeout handling of min_timeout and max_timeout
> v3:
>   - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
>   - move the watchdog_init_timeout function from inline to watchdog_core.c
>   - add timeout parameter handling code to watchdog_init_timeout
>   - add timeout-sec property to the pnx4008-wdt driver
>   - add two small commit for fixup and cleanup
> v2:
>   - change "timeout" to "timeout-sec" as asked by Jean-Christophe
>   - at91sam9_wdt: use the new helper function
>   - at91sam9_wdt: add bounds checking
>   - watchdog.h: add bounds checking
> 
> Fabio Porcedda (6):
>   watchdog: core: dt: add support for the timeout-sec dt property
>   watchdog: orion_wdt: add timeout-sec property binding
>   watchdog: pnx4008: add timeout-sec property binding
>   watchdog: s3c2410_wdt: add timeout-sec property binding
>   watchdog: at91sam9_wdt: add timeout-sec property binding
>   watchdog: orion_wdt: move the min_timeout initialization
> 
>  .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
>  .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
>  .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
>  .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
>  Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
>  drivers/watchdog/Kconfig                           |  1 +
>  drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
>  drivers/watchdog/orion_wdt.c                       | 10 +++---
>  drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
>  drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
>  drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
>  include/linux/watchdog.h                           |  3 ++
>  12 files changed, 91 insertions(+), 15 deletions(-)
> 
> -- 
> 1.8.0.3
> 

[PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:04 Tue 08 Jan     , Fabio Porcedda wrote:
> Hi all,
> this patchset add the timeout-sec property to the watchdog core,
> to the at91sam9_wdt driver and on all framework enabled watchdog driver
> with dt support.
> The at91sam9_wdt is tested on evk-pr3,
> the other drivers are compile tested only.
> 

I'll prefer a macro to abstract the string in every driver

except this fine by me

Best Regards,
J.

> Best regards
> Fabio Porcedda
> 
> changes:
> v7:
>   - rebase over v3.8-rc2
>   - remove the patch already merged
> v6:
>   - remove patch for function of_get_timeout_sec as aked by Rob Herring
>   - add Acked-by: Jason Cooper <jason@lakedaemon.net>
>   - add Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> v5:
>   - fix codestyle for compilance ti checkpatch.pl about
>     "ERROR: do not initialise statics to 0 or NULL"
>     for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
> v4:
>   - add of_get_timeout_sec
>   - add s3c2410_wdt driver
>   - split driver patch
>   - fixed watchdog_init_timeout handling of min_timeout and max_timeout
> v3:
>   - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
>   - move the watchdog_init_timeout function from inline to watchdog_core.c
>   - add timeout parameter handling code to watchdog_init_timeout
>   - add timeout-sec property to the pnx4008-wdt driver
>   - add two small commit for fixup and cleanup
> v2:
>   - change "timeout" to "timeout-sec" as asked by Jean-Christophe
>   - at91sam9_wdt: use the new helper function
>   - at91sam9_wdt: add bounds checking
>   - watchdog.h: add bounds checking
> 
> Fabio Porcedda (6):
>   watchdog: core: dt: add support for the timeout-sec dt property
>   watchdog: orion_wdt: add timeout-sec property binding
>   watchdog: pnx4008: add timeout-sec property binding
>   watchdog: s3c2410_wdt: add timeout-sec property binding
>   watchdog: at91sam9_wdt: add timeout-sec property binding
>   watchdog: orion_wdt: move the min_timeout initialization
> 
>  .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
>  .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
>  .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
>  .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
>  Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
>  drivers/watchdog/Kconfig                           |  1 +
>  drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
>  drivers/watchdog/orion_wdt.c                       | 10 +++---
>  drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
>  drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
>  drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
>  include/linux/watchdog.h                           |  3 ++
>  12 files changed, 91 insertions(+), 15 deletions(-)
> 
> -- 
> 1.8.0.3
> 

Re: [PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Tue, Jan 8, 2013 at 4:34 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org> wrote:
> On 11:04 Tue 08 Jan     , Fabio Porcedda wrote:
>> Hi all,
>> this patchset add the timeout-sec property to the watchdog core,
>> to the at91sam9_wdt driver and on all framework enabled watchdog driver
>> with dt support.
>> The at91sam9_wdt is tested on evk-pr3,
>> the other drivers are compile tested only.
>>
>
> I'll prefer a macro to abstract the string in every driver

I know but Rob Herring dislikes that macro/function. I had to remove that part.

> except this fine by me

On Fri, Dec 21, 2012 at 12:32 PM, Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org> wrote:
>> Ping.
>>
>> The dependency is already merged.
>
> Will not be for this merge window. Will review next week.

Hi Wim,

Ping.

Best regards
Fabio Porcedda

> Best Regards,
> J.
>
>> Best regards
>> Fabio Porcedda
>>
>> changes:
>> v7:
>>   - rebase over v3.8-rc2
>>   - remove the patch already merged
>> v6:
>>   - remove patch for function of_get_timeout_sec as aked by Rob Herring
>>   - add Acked-by: Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>
>>   - add Acked-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> v5:
>>   - fix codestyle for compilance ti checkpatch.pl about
>>     "ERROR: do not initialise statics to 0 or NULL"
>>     for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
>> v4:
>>   - add of_get_timeout_sec
>>   - add s3c2410_wdt driver
>>   - split driver patch
>>   - fixed watchdog_init_timeout handling of min_timeout and max_timeout
>> v3:
>>   - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
>>   - move the watchdog_init_timeout function from inline to watchdog_core.c
>>   - add timeout parameter handling code to watchdog_init_timeout
>>   - add timeout-sec property to the pnx4008-wdt driver
>>   - add two small commit for fixup and cleanup
>> v2:
>>   - change "timeout" to "timeout-sec" as asked by Jean-Christophe
>>   - at91sam9_wdt: use the new helper function
>>   - at91sam9_wdt: add bounds checking
>>   - watchdog.h: add bounds checking
>>
>> Fabio Porcedda (6):
>>   watchdog: core: dt: add support for the timeout-sec dt property
>>   watchdog: orion_wdt: add timeout-sec property binding
>>   watchdog: pnx4008: add timeout-sec property binding
>>   watchdog: s3c2410_wdt: add timeout-sec property binding
>>   watchdog: at91sam9_wdt: add timeout-sec property binding
>>   watchdog: orion_wdt: move the min_timeout initialization
>>
>>  .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
>>  .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
>>  .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
>>  .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
>>  Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
>>  drivers/watchdog/Kconfig                           |  1 +
>>  drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
>>  drivers/watchdog/orion_wdt.c                       | 10 +++---
>>  drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
>>  drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
>>  drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
>>  include/linux/watchdog.h                           |  3 ++
>>  12 files changed, 91 insertions(+), 15 deletions(-)
>>
>> --
>> 1.8.0.3
>>



--
Fabio Porcedda

Re: [PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Tue, Jan 8, 2013 at 4:34 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 11:04 Tue 08 Jan     , Fabio Porcedda wrote:
>> Hi all,
>> this patchset add the timeout-sec property to the watchdog core,
>> to the at91sam9_wdt driver and on all framework enabled watchdog driver
>> with dt support.
>> The at91sam9_wdt is tested on evk-pr3,
>> the other drivers are compile tested only.
>>
>
> I'll prefer a macro to abstract the string in every driver

I know but Rob Herring dislikes that macro/function. I had to remove that part.

> except this fine by me

On Fri, Dec 21, 2012 at 12:32 PM, Wim Van Sebroeck <wim@iguana.be> wrote:
>> Ping.
>>
>> The dependency is already merged.
>
> Will not be for this merge window. Will review next week.

Hi Wim,

Ping.

Best regards
Fabio Porcedda

> Best Regards,
> J.
>
>> Best regards
>> Fabio Porcedda
>>
>> changes:
>> v7:
>>   - rebase over v3.8-rc2
>>   - remove the patch already merged
>> v6:
>>   - remove patch for function of_get_timeout_sec as aked by Rob Herring
>>   - add Acked-by: Jason Cooper <jason@lakedaemon.net>
>>   - add Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> v5:
>>   - fix codestyle for compilance ti checkpatch.pl about
>>     "ERROR: do not initialise statics to 0 or NULL"
>>     for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
>> v4:
>>   - add of_get_timeout_sec
>>   - add s3c2410_wdt driver
>>   - split driver patch
>>   - fixed watchdog_init_timeout handling of min_timeout and max_timeout
>> v3:
>>   - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
>>   - move the watchdog_init_timeout function from inline to watchdog_core.c
>>   - add timeout parameter handling code to watchdog_init_timeout
>>   - add timeout-sec property to the pnx4008-wdt driver
>>   - add two small commit for fixup and cleanup
>> v2:
>>   - change "timeout" to "timeout-sec" as asked by Jean-Christophe
>>   - at91sam9_wdt: use the new helper function
>>   - at91sam9_wdt: add bounds checking
>>   - watchdog.h: add bounds checking
>>
>> Fabio Porcedda (6):
>>   watchdog: core: dt: add support for the timeout-sec dt property
>>   watchdog: orion_wdt: add timeout-sec property binding
>>   watchdog: pnx4008: add timeout-sec property binding
>>   watchdog: s3c2410_wdt: add timeout-sec property binding
>>   watchdog: at91sam9_wdt: add timeout-sec property binding
>>   watchdog: orion_wdt: move the min_timeout initialization
>>
>>  .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
>>  .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
>>  .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
>>  .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
>>  Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
>>  drivers/watchdog/Kconfig                           |  1 +
>>  drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
>>  drivers/watchdog/orion_wdt.c                       | 10 +++---
>>  drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
>>  drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
>>  drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
>>  include/linux/watchdog.h                           |  3 ++
>>  12 files changed, 91 insertions(+), 15 deletions(-)
>>
>> --
>> 1.8.0.3
>>



--
Fabio Porcedda

[PATCH v7 0/6] watchdog: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Tue, Jan 8, 2013 at 4:34 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 11:04 Tue 08 Jan     , Fabio Porcedda wrote:
>> Hi all,
>> this patchset add the timeout-sec property to the watchdog core,
>> to the at91sam9_wdt driver and on all framework enabled watchdog driver
>> with dt support.
>> The at91sam9_wdt is tested on evk-pr3,
>> the other drivers are compile tested only.
>>
>
> I'll prefer a macro to abstract the string in every driver

I know but Rob Herring dislikes that macro/function. I had to remove that part.

> except this fine by me

On Fri, Dec 21, 2012 at 12:32 PM, Wim Van Sebroeck <wim@iguana.be> wrote:
>> Ping.
>>
>> The dependency is already merged.
>
> Will not be for this merge window. Will review next week.

Hi Wim,

Ping.

Best regards
Fabio Porcedda

> Best Regards,
> J.
>
>> Best regards
>> Fabio Porcedda
>>
>> changes:
>> v7:
>>   - rebase over v3.8-rc2
>>   - remove the patch already merged
>> v6:
>>   - remove patch for function of_get_timeout_sec as aked by Rob Herring
>>   - add Acked-by: Jason Cooper <jason@lakedaemon.net>
>>   - add Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>> v5:
>>   - fix codestyle for compilance ti checkpatch.pl about
>>     "ERROR: do not initialise statics to 0 or NULL"
>>     for s3c2410_wdt.c:56 and at91sam9_wdt.c:63
>> v4:
>>   - add of_get_timeout_sec
>>   - add s3c2410_wdt driver
>>   - split driver patch
>>   - fixed watchdog_init_timeout handling of min_timeout and max_timeout
>> v3:
>>   - rename watchdog_probe_dt_timeout ->  watchdog_init_timeout
>>   - move the watchdog_init_timeout function from inline to watchdog_core.c
>>   - add timeout parameter handling code to watchdog_init_timeout
>>   - add timeout-sec property to the pnx4008-wdt driver
>>   - add two small commit for fixup and cleanup
>> v2:
>>   - change "timeout" to "timeout-sec" as asked by Jean-Christophe
>>   - at91sam9_wdt: use the new helper function
>>   - at91sam9_wdt: add bounds checking
>>   - watchdog.h: add bounds checking
>>
>> Fabio Porcedda (6):
>>   watchdog: core: dt: add support for the timeout-sec dt property
>>   watchdog: orion_wdt: add timeout-sec property binding
>>   watchdog: pnx4008: add timeout-sec property binding
>>   watchdog: s3c2410_wdt: add timeout-sec property binding
>>   watchdog: at91sam9_wdt: add timeout-sec property binding
>>   watchdog: orion_wdt: move the min_timeout initialization
>>
>>  .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 +++
>>  .../devicetree/bindings/watchdog/marvel.txt        |  5 +++
>>  .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 +++
>>  .../devicetree/bindings/watchdog/samsung-wdt.txt   |  3 ++
>>  Documentation/watchdog/watchdog-kernel-api.txt     | 10 ++++++
>>  drivers/watchdog/Kconfig                           |  1 +
>>  drivers/watchdog/at91sam9_wdt.c                    | 16 ++++++++--
>>  drivers/watchdog/orion_wdt.c                       | 10 +++---
>>  drivers/watchdog/pnx4008_wdt.c                     |  7 ++--
>>  drivers/watchdog/s3c2410_wdt.c                     |  6 ++--
>>  drivers/watchdog/watchdog_core.c                   | 37 ++++++++++++++++++++++
>>  include/linux/watchdog.h                           |  3 ++
>>  12 files changed, 91 insertions(+), 15 deletions(-)
>>
>> --
>> 1.8.0.3
>>



--
Fabio Porcedda

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Wim Van Sebroeck]

Hi Fabio,

I must admit that the different iterations of this patch only made it better.
I like the idea that you are proposing here (put the default timeout in the
watchdog_device struct and use a helper function to set the timeout parameter
value or the timeout-sec dt value). To be able to use other mechanism in the
future also, I think it make more sense to pass the device instead of the
device_node.

> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
>  drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
>  include/linux/watchdog.h                       |  3 +++
>  3 files changed, 50 insertions(+)
> 
...
>  
> +static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
> +				      unsigned int t)
> +
> +{
> +	if (wdd->min_timeout < wdd->max_timeout)
> +		return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
> +	else
> +		return (t > 0);
> +}

We use a similar check allready in watchdog_dev.c in the set_timeout wrapper.
And we should move this to the watchdog.h include file so that drivers can also
use the logic if necessary.

> +/**
> + * watchdog_init_timeout() - initialize the timeout field
> + * @parm: timeout module parameter, it takes precedence over the
> + *        timeout-sec property.
> + * @node: Retrieve the timeout-sec property only if the parm_timeout
> + *        is out of bounds.
> + */
> +void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
> +			   struct device_node *node)

make this int so that drivers can give appropriate warnings if necessary.

I also detailed your documentation a bit more and incorporated above changes
in an adjusted patch. Can you have a look at it and if OK for you I will put
it in linux-watchdog-next as your v10 :-).

Kind regards,
Wim.
---
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..a0438f3 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -1,6 +1,6 @@
 The Linux WatchDog Timer Driver Core kernel API.
 ===============================================
-Last reviewed: 22-May-2012
+Last reviewed: 12-Feb-2013
 
 Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
 
@@ -212,3 +212,15 @@ driver specific data to and a pointer to the data itself.
 The watchdog_get_drvdata function allows you to retrieve driver specific data.
 The argument of this function is the watchdog device where you want to retrieve
 data from. The function returns the pointer to the driver specific data.
+
+To initialize the timeout field, the following function can be used:
+
+extern int watchdog_init_timeout(struct watchdog_device *wdd,
+                                  unsigned int timeout_parm, struct device *dev);
+
+The watchdog_init_timeout function allows you to initialize the timeout field
+using the module timeout parameter or by retrieving the timeout-sec property from
+the device tree (if the module timeout parameter is invalid). Best practice is
+to set the default timeout value as timeout value in the watchdog_device and
+then use this function to set the user "preferred" timeout value.
+This routine returns zero on success and a negative errno code for failure.
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 3796434..a74a3ef 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -36,12 +36,62 @@
 #include <linux/init.h>		/* For __init/__exit/... */
 #include <linux/idr.h>		/* For ida_* macros */
 #include <linux/err.h>		/* For IS_ERR macros */
+#include <linux/of.h>		/* For of_get_timeout_sec */
 
 #include "watchdog_core.h"	/* For watchdog_dev_register/... */
 
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
+{
+	/*
+	 * Check that we have valid min and max timeout values, if
+	 * not reset them both to 0 (=not used or unknown)
+	 */
+	if (wdd->min_timeout > wdd->max_timeout) {
+		pr_info("Invalid min and max timeout values, resetting to 0!\n");
+		wdd->min_timeout = 0;
+		wdd->max_timeout = 0;
+	}
+}
+
+/**
+ * watchdog_init_timeout() - initialize the timeout field
+ * @timeout_parm: timeout module parameter
+ * @dev: Device that stores the timeout-sec property
+ *
+ * Initialize the timeout field of the watchdog_device struct with either the
+ * timeout module parameter (if it is valid value) or the timeout-sec property
+ * (only if it is a valid value and the timeout_parm is out of bounds).
+ * If none of them are valid then we keep the old value (which should normally
+ * be the default timeout value.
+ *
+ * A zero is returned on success and -EINVAL for failure.
+ */
+int watchdog_init_timeout(struct watchdog_device *wdd,
+				unsigned int timeout_parm, struct device *dev)
+{
+	unsigned int t = 0;
+
+	watchdog_check_min_max_timeout(wdd);
+
+	/* try to get the tiemout module parameter first */
+	if (!watchdog_timeout_invalid(wdd, timeout_parm)) {
+		wdd->timeout = timeout_parm;
+		return 0;
+	}
+
+	/* try to get the timeout_sec property */
+	if (dev == NULL || dev->of_node == NULL)
+		return -EINVAL;
+	of_property_read_u32(dev->of_node, "timeout-sec", &t);
+	if (!watchdog_timeout_invalid(wdd, t))
+		wdd->timeout = t;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+
 /**
  * watchdog_register_device() - register a watchdog device
  * @wdd: watchdog device
@@ -63,15 +113,7 @@ int watchdog_register_device(struct watchdog_device *wdd)
 	if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
 		return -EINVAL;
 
-	/*
-	 * Check that we have valid min and max timeout values, if
-	 * not reset them both to 0 (=not used or unknown)
-	 */
-	if (wdd->min_timeout > wdd->max_timeout) {
-		pr_info("Invalid min and max timeout values, resetting to 0!\n");
-		wdd->min_timeout = 0;
-		wdd->max_timeout = 0;
-	}
+	watchdog_check_min_max_timeout(wdd);
 
 	/*
 	 * Note: now that all watchdog_device data has been verified, we
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index ef8edec..08b48bb 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -200,8 +200,7 @@ static int watchdog_set_timeout(struct watchdog_device *wddev,
 	    !(wddev->info->options & WDIOF_SETTIMEOUT))
 		return -EOPNOTSUPP;
 
-	if ((wddev->max_timeout != 0) &&
-	    (timeout < wddev->min_timeout || timeout > wddev->max_timeout))
+	if (watchdog_timeout_invalid(wddev, timeout))
 		return -EINVAL;
 
 	mutex_lock(&wddev->lock);
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 3a9df2f..2a3038e 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -118,6 +118,13 @@ static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway
 		set_bit(WDOG_NO_WAY_OUT, &wdd->status);
 }
 
+/* Use the following function to check if a timeout value is invalid */
+static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t)
+{
+	return ((wdd->max_timeout != 0) &&
+		(t < wdd->min_timeout || t > wdd->max_timeout));
+}
+
 /* Use the following functions to manipulate watchdog driver specific data */
 static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
 {
@@ -130,6 +137,8 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 }
 
 /* drivers/watchdog/watchdog_core.c */
+extern int watchdog_init_timeout(struct watchdog_device *wdd,
+				  unsigned int timeout_parm, struct device *dev);
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Wim Van Sebroeck]

Hi Fabio,

I must admit that the different iterations of this patch only made it better.
I like the idea that you are proposing here (put the default timeout in the
watchdog_device struct and use a helper function to set the timeout parameter
value or the timeout-sec dt value). To be able to use other mechanism in the
future also, I think it make more sense to pass the device instead of the
device_node.

> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
>  drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
>  include/linux/watchdog.h                       |  3 +++
>  3 files changed, 50 insertions(+)
> 
...
>  
> +static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
> +				      unsigned int t)
> +
> +{
> +	if (wdd->min_timeout < wdd->max_timeout)
> +		return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
> +	else
> +		return (t > 0);
> +}

We use a similar check allready in watchdog_dev.c in the set_timeout wrapper.
And we should move this to the watchdog.h include file so that drivers can also
use the logic if necessary.

> +/**
> + * watchdog_init_timeout() - initialize the timeout field
> + * @parm: timeout module parameter, it takes precedence over the
> + *        timeout-sec property.
> + * @node: Retrieve the timeout-sec property only if the parm_timeout
> + *        is out of bounds.
> + */
> +void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
> +			   struct device_node *node)

make this int so that drivers can give appropriate warnings if necessary.

I also detailed your documentation a bit more and incorporated above changes
in an adjusted patch. Can you have a look at it and if OK for you I will put
it in linux-watchdog-next as your v10 :-).

Kind regards,
Wim.
---
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..a0438f3 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -1,6 +1,6 @@
 The Linux WatchDog Timer Driver Core kernel API.
 ===============================================
-Last reviewed: 22-May-2012
+Last reviewed: 12-Feb-2013
 
 Wim Van Sebroeck <wim@iguana.be>
 
@@ -212,3 +212,15 @@ driver specific data to and a pointer to the data itself.
 The watchdog_get_drvdata function allows you to retrieve driver specific data.
 The argument of this function is the watchdog device where you want to retrieve
 data from. The function returns the pointer to the driver specific data.
+
+To initialize the timeout field, the following function can be used:
+
+extern int watchdog_init_timeout(struct watchdog_device *wdd,
+                                  unsigned int timeout_parm, struct device *dev);
+
+The watchdog_init_timeout function allows you to initialize the timeout field
+using the module timeout parameter or by retrieving the timeout-sec property from
+the device tree (if the module timeout parameter is invalid). Best practice is
+to set the default timeout value as timeout value in the watchdog_device and
+then use this function to set the user "preferred" timeout value.
+This routine returns zero on success and a negative errno code for failure.
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 3796434..a74a3ef 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -36,12 +36,62 @@
 #include <linux/init.h>		/* For __init/__exit/... */
 #include <linux/idr.h>		/* For ida_* macros */
 #include <linux/err.h>		/* For IS_ERR macros */
+#include <linux/of.h>		/* For of_get_timeout_sec */
 
 #include "watchdog_core.h"	/* For watchdog_dev_register/... */
 
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
+{
+	/*
+	 * Check that we have valid min and max timeout values, if
+	 * not reset them both to 0 (=not used or unknown)
+	 */
+	if (wdd->min_timeout > wdd->max_timeout) {
+		pr_info("Invalid min and max timeout values, resetting to 0!\n");
+		wdd->min_timeout = 0;
+		wdd->max_timeout = 0;
+	}
+}
+
+/**
+ * watchdog_init_timeout() - initialize the timeout field
+ * @timeout_parm: timeout module parameter
+ * @dev: Device that stores the timeout-sec property
+ *
+ * Initialize the timeout field of the watchdog_device struct with either the
+ * timeout module parameter (if it is valid value) or the timeout-sec property
+ * (only if it is a valid value and the timeout_parm is out of bounds).
+ * If none of them are valid then we keep the old value (which should normally
+ * be the default timeout value.
+ *
+ * A zero is returned on success and -EINVAL for failure.
+ */
+int watchdog_init_timeout(struct watchdog_device *wdd,
+				unsigned int timeout_parm, struct device *dev)
+{
+	unsigned int t = 0;
+
+	watchdog_check_min_max_timeout(wdd);
+
+	/* try to get the tiemout module parameter first */
+	if (!watchdog_timeout_invalid(wdd, timeout_parm)) {
+		wdd->timeout = timeout_parm;
+		return 0;
+	}
+
+	/* try to get the timeout_sec property */
+	if (dev == NULL || dev->of_node == NULL)
+		return -EINVAL;
+	of_property_read_u32(dev->of_node, "timeout-sec", &t);
+	if (!watchdog_timeout_invalid(wdd, t))
+		wdd->timeout = t;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+
 /**
  * watchdog_register_device() - register a watchdog device
  * @wdd: watchdog device
@@ -63,15 +113,7 @@ int watchdog_register_device(struct watchdog_device *wdd)
 	if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
 		return -EINVAL;
 
-	/*
-	 * Check that we have valid min and max timeout values, if
-	 * not reset them both to 0 (=not used or unknown)
-	 */
-	if (wdd->min_timeout > wdd->max_timeout) {
-		pr_info("Invalid min and max timeout values, resetting to 0!\n");
-		wdd->min_timeout = 0;
-		wdd->max_timeout = 0;
-	}
+	watchdog_check_min_max_timeout(wdd);
 
 	/*
 	 * Note: now that all watchdog_device data has been verified, we
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index ef8edec..08b48bb 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -200,8 +200,7 @@ static int watchdog_set_timeout(struct watchdog_device *wddev,
 	    !(wddev->info->options & WDIOF_SETTIMEOUT))
 		return -EOPNOTSUPP;
 
-	if ((wddev->max_timeout != 0) &&
-	    (timeout < wddev->min_timeout || timeout > wddev->max_timeout))
+	if (watchdog_timeout_invalid(wddev, timeout))
 		return -EINVAL;
 
 	mutex_lock(&wddev->lock);
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 3a9df2f..2a3038e 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -118,6 +118,13 @@ static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway
 		set_bit(WDOG_NO_WAY_OUT, &wdd->status);
 }
 
+/* Use the following function to check if a timeout value is invalid */
+static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t)
+{
+	return ((wdd->max_timeout != 0) &&
+		(t < wdd->min_timeout || t > wdd->max_timeout));
+}
+
 /* Use the following functions to manipulate watchdog driver specific data */
 static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
 {
@@ -130,6 +137,8 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 }
 
 /* drivers/watchdog/watchdog_core.c */
+extern int watchdog_init_timeout(struct watchdog_device *wdd,
+				  unsigned int timeout_parm, struct device *dev);
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Wed, Feb 13, 2013 at 12:03 AM, Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org> wrote:
> Hi Fabio,
>
> I must admit that the different iterations of this patch only made it better.
> I like the idea that you are proposing here (put the default timeout in the
> watchdog_device struct and use a helper function to set the timeout parameter
> value or the timeout-sec dt value). To be able to use other mechanism in the
> future also, I think it make more sense to pass the device instead of the
> device_node.

If i understand correctly you want to use "struct platform_device"
instead of "struct device_node",
in the function watchdog_init_timeout?

>> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
>>  drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
>>  include/linux/watchdog.h                       |  3 +++
>>  3 files changed, 50 insertions(+)
>>
> ...
>>
>> +static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
>> +                                   unsigned int t)
>> +
>> +{
>> +     if (wdd->min_timeout < wdd->max_timeout)
>> +             return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
>> +     else
>> +             return (t > 0);
>> +}
>
> We use a similar check allready in watchdog_dev.c in the set_timeout wrapper.
> And we should move this to the watchdog.h include file so that drivers can also
> use the logic if necessary.

Do you prefer that i move or just export that function in watchdog.h?
I think it's better to just export it because the function is
not small enough as inline function, but both solutions are fine to me.

>
>> +/**
>> + * watchdog_init_timeout() - initialize the timeout field
>> + * @parm: timeout module parameter, it takes precedence over the
>> + *        timeout-sec property.
>> + * @node: Retrieve the timeout-sec property only if the parm_timeout
>> + *        is out of bounds.
>> + */
>> +void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
>> +                        struct device_node *node)
>
> make this int so that drivers can give appropriate warnings if necessary.

In which cases i must return a non zero result?
For a value out of range in parm or node?
Wich value -EINVAL?


> I also detailed your documentation a bit more and incorporated above changes
> in an adjusted patch. Can you have a look at it and if OK for you I will put
> it in linux-watchdog-next as your v10 :-).

That's good for sure :)

> Kind regards,
> Wim.
> ---
> diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
> index 086638f..a0438f3 100644
> --- a/Documentation/watchdog/watchdog-kernel-api.txt
> +++ b/Documentation/watchdog/watchdog-kernel-api.txt
> @@ -1,6 +1,6 @@
>  The Linux WatchDog Timer Driver Core kernel API.
>  ===============================================
> -Last reviewed: 22-May-2012
> +Last reviewed: 12-Feb-2013
>
>  Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org>
>
> @@ -212,3 +212,15 @@ driver specific data to and a pointer to the data itself.
>  The watchdog_get_drvdata function allows you to retrieve driver specific data.
>  The argument of this function is the watchdog device where you want to retrieve
>  data from. The function returns the pointer to the driver specific data.
> +
> +To initialize the timeout field, the following function can be used:
> +
> +extern int watchdog_init_timeout(struct watchdog_device *wdd,
> +                                  unsigned int timeout_parm, struct device *dev);
> +
> +The watchdog_init_timeout function allows you to initialize the timeout field
> +using the module timeout parameter or by retrieving the timeout-sec property from
> +the device tree (if the module timeout parameter is invalid). Best practice is
> +to set the default timeout value as timeout value in the watchdog_device and
> +then use this function to set the user "preferred" timeout value.
> +This routine returns zero on success and a negative errno code for failure.
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 3796434..a74a3ef 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -36,12 +36,62 @@
>  #include <linux/init.h>                /* For __init/__exit/... */
>  #include <linux/idr.h>         /* For ida_* macros */
>  #include <linux/err.h>         /* For IS_ERR macros */
> +#include <linux/of.h>          /* For of_get_timeout_sec */
>
>  #include "watchdog_core.h"     /* For watchdog_dev_register/... */
>
>  static DEFINE_IDA(watchdog_ida);
>  static struct class *watchdog_class;
>
> +void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
> +{
> +       /*
> +        * Check that we have valid min and max timeout values, if
> +        * not reset them both to 0 (=not used or unknown)
> +        */
> +       if (wdd->min_timeout > wdd->max_timeout) {
> +               pr_info("Invalid min and max timeout values, resetting to 0!\n");
> +               wdd->min_timeout = 0;
> +               wdd->max_timeout = 0;
> +       }
> +}
> +
> +/**
> + * watchdog_init_timeout() - initialize the timeout field
> + * @timeout_parm: timeout module parameter
> + * @dev: Device that stores the timeout-sec property
> + *
> + * Initialize the timeout field of the watchdog_device struct with either the
> + * timeout module parameter (if it is valid value) or the timeout-sec property
> + * (only if it is a valid value and the timeout_parm is out of bounds).
> + * If none of them are valid then we keep the old value (which should normally
> + * be the default timeout value.
> + *
> + * A zero is returned on success and -EINVAL for failure.
> + */
> +int watchdog_init_timeout(struct watchdog_device *wdd,
> +                               unsigned int timeout_parm, struct device *dev)
> +{
> +       unsigned int t = 0;
> +
> +       watchdog_check_min_max_timeout(wdd);
> +
> +       /* try to get the tiemout module parameter first */
> +       if (!watchdog_timeout_invalid(wdd, timeout_parm)) {
> +               wdd->timeout = timeout_parm;
> +               return 0;
> +       }
> +
> +       /* try to get the timeout_sec property */
> +       if (dev == NULL || dev->of_node == NULL)
> +               return -EINVAL;
> +       of_property_read_u32(dev->of_node, "timeout-sec", &t);
> +       if (!watchdog_timeout_invalid(wdd, t))
> +               wdd->timeout = t;
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_init_timeout);
> +
>  /**
>   * watchdog_register_device() - register a watchdog device
>   * @wdd: watchdog device
> @@ -63,15 +113,7 @@ int watchdog_register_device(struct watchdog_device *wdd)
>         if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
>                 return -EINVAL;
>
> -       /*
> -        * Check that we have valid min and max timeout values, if
> -        * not reset them both to 0 (=not used or unknown)
> -        */
> -       if (wdd->min_timeout > wdd->max_timeout) {
> -               pr_info("Invalid min and max timeout values, resetting to 0!\n");
> -               wdd->min_timeout = 0;
> -               wdd->max_timeout = 0;
> -       }
> +       watchdog_check_min_max_timeout(wdd);
>
>         /*
>          * Note: now that all watchdog_device data has been verified, we
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index ef8edec..08b48bb 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -200,8 +200,7 @@ static int watchdog_set_timeout(struct watchdog_device *wddev,
>             !(wddev->info->options & WDIOF_SETTIMEOUT))
>                 return -EOPNOTSUPP;
>
> -       if ((wddev->max_timeout != 0) &&
> -           (timeout < wddev->min_timeout || timeout > wddev->max_timeout))
> +       if (watchdog_timeout_invalid(wddev, timeout))
>                 return -EINVAL;
>
>         mutex_lock(&wddev->lock);
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 3a9df2f..2a3038e 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -118,6 +118,13 @@ static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway
>                 set_bit(WDOG_NO_WAY_OUT, &wdd->status);
>  }
>
> +/* Use the following function to check if a timeout value is invalid */
> +static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t)
> +{
> +       return ((wdd->max_timeout != 0) &&
> +               (t < wdd->min_timeout || t > wdd->max_timeout));
> +}
> +
>  /* Use the following functions to manipulate watchdog driver specific data */
>  static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
>  {
> @@ -130,6 +137,8 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
>  }
>
>  /* drivers/watchdog/watchdog_core.c */
> +extern int watchdog_init_timeout(struct watchdog_device *wdd,
> +                                 unsigned int timeout_parm, struct device *dev);
>  extern int watchdog_register_device(struct watchdog_device *);
>  extern void watchdog_unregister_device(struct watchdog_device *);
>



--
Fabio Porcedda

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Wed, Feb 13, 2013 at 12:03 AM, Wim Van Sebroeck <wim@iguana.be> wrote:
> Hi Fabio,
>
> I must admit that the different iterations of this patch only made it better.
> I like the idea that you are proposing here (put the default timeout in the
> watchdog_device struct and use a helper function to set the timeout parameter
> value or the timeout-sec dt value). To be able to use other mechanism in the
> future also, I think it make more sense to pass the device instead of the
> device_node.

If i understand correctly you want to use "struct platform_device"
instead of "struct device_node",
in the function watchdog_init_timeout?

>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>  Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
>>  drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
>>  include/linux/watchdog.h                       |  3 +++
>>  3 files changed, 50 insertions(+)
>>
> ...
>>
>> +static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
>> +                                   unsigned int t)
>> +
>> +{
>> +     if (wdd->min_timeout < wdd->max_timeout)
>> +             return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
>> +     else
>> +             return (t > 0);
>> +}
>
> We use a similar check allready in watchdog_dev.c in the set_timeout wrapper.
> And we should move this to the watchdog.h include file so that drivers can also
> use the logic if necessary.

Do you prefer that i move or just export that function in watchdog.h?
I think it's better to just export it because the function is
not small enough as inline function, but both solutions are fine to me.

>
>> +/**
>> + * watchdog_init_timeout() - initialize the timeout field
>> + * @parm: timeout module parameter, it takes precedence over the
>> + *        timeout-sec property.
>> + * @node: Retrieve the timeout-sec property only if the parm_timeout
>> + *        is out of bounds.
>> + */
>> +void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
>> +                        struct device_node *node)
>
> make this int so that drivers can give appropriate warnings if necessary.

In which cases i must return a non zero result?
For a value out of range in parm or node?
Wich value -EINVAL?


> I also detailed your documentation a bit more and incorporated above changes
> in an adjusted patch. Can you have a look at it and if OK for you I will put
> it in linux-watchdog-next as your v10 :-).

That's good for sure :)

> Kind regards,
> Wim.
> ---
> diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
> index 086638f..a0438f3 100644
> --- a/Documentation/watchdog/watchdog-kernel-api.txt
> +++ b/Documentation/watchdog/watchdog-kernel-api.txt
> @@ -1,6 +1,6 @@
>  The Linux WatchDog Timer Driver Core kernel API.
>  ===============================================
> -Last reviewed: 22-May-2012
> +Last reviewed: 12-Feb-2013
>
>  Wim Van Sebroeck <wim@iguana.be>
>
> @@ -212,3 +212,15 @@ driver specific data to and a pointer to the data itself.
>  The watchdog_get_drvdata function allows you to retrieve driver specific data.
>  The argument of this function is the watchdog device where you want to retrieve
>  data from. The function returns the pointer to the driver specific data.
> +
> +To initialize the timeout field, the following function can be used:
> +
> +extern int watchdog_init_timeout(struct watchdog_device *wdd,
> +                                  unsigned int timeout_parm, struct device *dev);
> +
> +The watchdog_init_timeout function allows you to initialize the timeout field
> +using the module timeout parameter or by retrieving the timeout-sec property from
> +the device tree (if the module timeout parameter is invalid). Best practice is
> +to set the default timeout value as timeout value in the watchdog_device and
> +then use this function to set the user "preferred" timeout value.
> +This routine returns zero on success and a negative errno code for failure.
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 3796434..a74a3ef 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -36,12 +36,62 @@
>  #include <linux/init.h>                /* For __init/__exit/... */
>  #include <linux/idr.h>         /* For ida_* macros */
>  #include <linux/err.h>         /* For IS_ERR macros */
> +#include <linux/of.h>          /* For of_get_timeout_sec */
>
>  #include "watchdog_core.h"     /* For watchdog_dev_register/... */
>
>  static DEFINE_IDA(watchdog_ida);
>  static struct class *watchdog_class;
>
> +void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
> +{
> +       /*
> +        * Check that we have valid min and max timeout values, if
> +        * not reset them both to 0 (=not used or unknown)
> +        */
> +       if (wdd->min_timeout > wdd->max_timeout) {
> +               pr_info("Invalid min and max timeout values, resetting to 0!\n");
> +               wdd->min_timeout = 0;
> +               wdd->max_timeout = 0;
> +       }
> +}
> +
> +/**
> + * watchdog_init_timeout() - initialize the timeout field
> + * @timeout_parm: timeout module parameter
> + * @dev: Device that stores the timeout-sec property
> + *
> + * Initialize the timeout field of the watchdog_device struct with either the
> + * timeout module parameter (if it is valid value) or the timeout-sec property
> + * (only if it is a valid value and the timeout_parm is out of bounds).
> + * If none of them are valid then we keep the old value (which should normally
> + * be the default timeout value.
> + *
> + * A zero is returned on success and -EINVAL for failure.
> + */
> +int watchdog_init_timeout(struct watchdog_device *wdd,
> +                               unsigned int timeout_parm, struct device *dev)
> +{
> +       unsigned int t = 0;
> +
> +       watchdog_check_min_max_timeout(wdd);
> +
> +       /* try to get the tiemout module parameter first */
> +       if (!watchdog_timeout_invalid(wdd, timeout_parm)) {
> +               wdd->timeout = timeout_parm;
> +               return 0;
> +       }
> +
> +       /* try to get the timeout_sec property */
> +       if (dev == NULL || dev->of_node == NULL)
> +               return -EINVAL;
> +       of_property_read_u32(dev->of_node, "timeout-sec", &t);
> +       if (!watchdog_timeout_invalid(wdd, t))
> +               wdd->timeout = t;
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_init_timeout);
> +
>  /**
>   * watchdog_register_device() - register a watchdog device
>   * @wdd: watchdog device
> @@ -63,15 +113,7 @@ int watchdog_register_device(struct watchdog_device *wdd)
>         if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
>                 return -EINVAL;
>
> -       /*
> -        * Check that we have valid min and max timeout values, if
> -        * not reset them both to 0 (=not used or unknown)
> -        */
> -       if (wdd->min_timeout > wdd->max_timeout) {
> -               pr_info("Invalid min and max timeout values, resetting to 0!\n");
> -               wdd->min_timeout = 0;
> -               wdd->max_timeout = 0;
> -       }
> +       watchdog_check_min_max_timeout(wdd);
>
>         /*
>          * Note: now that all watchdog_device data has been verified, we
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index ef8edec..08b48bb 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -200,8 +200,7 @@ static int watchdog_set_timeout(struct watchdog_device *wddev,
>             !(wddev->info->options & WDIOF_SETTIMEOUT))
>                 return -EOPNOTSUPP;
>
> -       if ((wddev->max_timeout != 0) &&
> -           (timeout < wddev->min_timeout || timeout > wddev->max_timeout))
> +       if (watchdog_timeout_invalid(wddev, timeout))
>                 return -EINVAL;
>
>         mutex_lock(&wddev->lock);
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 3a9df2f..2a3038e 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -118,6 +118,13 @@ static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway
>                 set_bit(WDOG_NO_WAY_OUT, &wdd->status);
>  }
>
> +/* Use the following function to check if a timeout value is invalid */
> +static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t)
> +{
> +       return ((wdd->max_timeout != 0) &&
> +               (t < wdd->min_timeout || t > wdd->max_timeout));
> +}
> +
>  /* Use the following functions to manipulate watchdog driver specific data */
>  static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
>  {
> @@ -130,6 +137,8 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
>  }
>
>  /* drivers/watchdog/watchdog_core.c */
> +extern int watchdog_init_timeout(struct watchdog_device *wdd,
> +                                 unsigned int timeout_parm, struct device *dev);
>  extern int watchdog_register_device(struct watchdog_device *);
>  extern void watchdog_unregister_device(struct watchdog_device *);
>



--
Fabio Porcedda

[PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Wed, Feb 13, 2013 at 12:03 AM, Wim Van Sebroeck <wim@iguana.be> wrote:
> Hi Fabio,
>
> I must admit that the different iterations of this patch only made it better.
> I like the idea that you are proposing here (put the default timeout in the
> watchdog_device struct and use a helper function to set the timeout parameter
> value or the timeout-sec dt value). To be able to use other mechanism in the
> future also, I think it make more sense to pass the device instead of the
> device_node.

If i understand correctly you want to use "struct platform_device"
instead of "struct device_node",
in the function watchdog_init_timeout?

>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>  Documentation/watchdog/watchdog-kernel-api.txt | 10 +++++++
>>  drivers/watchdog/watchdog_core.c               | 37 ++++++++++++++++++++++++++
>>  include/linux/watchdog.h                       |  3 +++
>>  3 files changed, 50 insertions(+)
>>
> ...
>>
>> +static bool watchdog_is_valid_timeout(struct watchdog_device *wdd,
>> +                                   unsigned int t)
>> +
>> +{
>> +     if (wdd->min_timeout < wdd->max_timeout)
>> +             return (wdd->min_timeout <= t) && (t <= wdd->max_timeout);
>> +     else
>> +             return (t > 0);
>> +}
>
> We use a similar check allready in watchdog_dev.c in the set_timeout wrapper.
> And we should move this to the watchdog.h include file so that drivers can also
> use the logic if necessary.

Do you prefer that i move or just export that function in watchdog.h?
I think it's better to just export it because the function is
not small enough as inline function, but both solutions are fine to me.

>
>> +/**
>> + * watchdog_init_timeout() - initialize the timeout field
>> + * @parm: timeout module parameter, it takes precedence over the
>> + *        timeout-sec property.
>> + * @node: Retrieve the timeout-sec property only if the parm_timeout
>> + *        is out of bounds.
>> + */
>> +void watchdog_init_timeout(struct watchdog_device *wdd, unsigned int parm,
>> +                        struct device_node *node)
>
> make this int so that drivers can give appropriate warnings if necessary.

In which cases i must return a non zero result?
For a value out of range in parm or node?
Wich value -EINVAL?


> I also detailed your documentation a bit more and incorporated above changes
> in an adjusted patch. Can you have a look at it and if OK for you I will put
> it in linux-watchdog-next as your v10 :-).

That's good for sure :)

> Kind regards,
> Wim.
> ---
> diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
> index 086638f..a0438f3 100644
> --- a/Documentation/watchdog/watchdog-kernel-api.txt
> +++ b/Documentation/watchdog/watchdog-kernel-api.txt
> @@ -1,6 +1,6 @@
>  The Linux WatchDog Timer Driver Core kernel API.
>  ===============================================
> -Last reviewed: 22-May-2012
> +Last reviewed: 12-Feb-2013
>
>  Wim Van Sebroeck <wim@iguana.be>
>
> @@ -212,3 +212,15 @@ driver specific data to and a pointer to the data itself.
>  The watchdog_get_drvdata function allows you to retrieve driver specific data.
>  The argument of this function is the watchdog device where you want to retrieve
>  data from. The function returns the pointer to the driver specific data.
> +
> +To initialize the timeout field, the following function can be used:
> +
> +extern int watchdog_init_timeout(struct watchdog_device *wdd,
> +                                  unsigned int timeout_parm, struct device *dev);
> +
> +The watchdog_init_timeout function allows you to initialize the timeout field
> +using the module timeout parameter or by retrieving the timeout-sec property from
> +the device tree (if the module timeout parameter is invalid). Best practice is
> +to set the default timeout value as timeout value in the watchdog_device and
> +then use this function to set the user "preferred" timeout value.
> +This routine returns zero on success and a negative errno code for failure.
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 3796434..a74a3ef 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -36,12 +36,62 @@
>  #include <linux/init.h>                /* For __init/__exit/... */
>  #include <linux/idr.h>         /* For ida_* macros */
>  #include <linux/err.h>         /* For IS_ERR macros */
> +#include <linux/of.h>          /* For of_get_timeout_sec */
>
>  #include "watchdog_core.h"     /* For watchdog_dev_register/... */
>
>  static DEFINE_IDA(watchdog_ida);
>  static struct class *watchdog_class;
>
> +void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
> +{
> +       /*
> +        * Check that we have valid min and max timeout values, if
> +        * not reset them both to 0 (=not used or unknown)
> +        */
> +       if (wdd->min_timeout > wdd->max_timeout) {
> +               pr_info("Invalid min and max timeout values, resetting to 0!\n");
> +               wdd->min_timeout = 0;
> +               wdd->max_timeout = 0;
> +       }
> +}
> +
> +/**
> + * watchdog_init_timeout() - initialize the timeout field
> + * @timeout_parm: timeout module parameter
> + * @dev: Device that stores the timeout-sec property
> + *
> + * Initialize the timeout field of the watchdog_device struct with either the
> + * timeout module parameter (if it is valid value) or the timeout-sec property
> + * (only if it is a valid value and the timeout_parm is out of bounds).
> + * If none of them are valid then we keep the old value (which should normally
> + * be the default timeout value.
> + *
> + * A zero is returned on success and -EINVAL for failure.
> + */
> +int watchdog_init_timeout(struct watchdog_device *wdd,
> +                               unsigned int timeout_parm, struct device *dev)
> +{
> +       unsigned int t = 0;
> +
> +       watchdog_check_min_max_timeout(wdd);
> +
> +       /* try to get the tiemout module parameter first */
> +       if (!watchdog_timeout_invalid(wdd, timeout_parm)) {
> +               wdd->timeout = timeout_parm;
> +               return 0;
> +       }
> +
> +       /* try to get the timeout_sec property */
> +       if (dev == NULL || dev->of_node == NULL)
> +               return -EINVAL;
> +       of_property_read_u32(dev->of_node, "timeout-sec", &t);
> +       if (!watchdog_timeout_invalid(wdd, t))
> +               wdd->timeout = t;
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_init_timeout);
> +
>  /**
>   * watchdog_register_device() - register a watchdog device
>   * @wdd: watchdog device
> @@ -63,15 +113,7 @@ int watchdog_register_device(struct watchdog_device *wdd)
>         if (wdd->ops->start == NULL || wdd->ops->stop == NULL)
>                 return -EINVAL;
>
> -       /*
> -        * Check that we have valid min and max timeout values, if
> -        * not reset them both to 0 (=not used or unknown)
> -        */
> -       if (wdd->min_timeout > wdd->max_timeout) {
> -               pr_info("Invalid min and max timeout values, resetting to 0!\n");
> -               wdd->min_timeout = 0;
> -               wdd->max_timeout = 0;
> -       }
> +       watchdog_check_min_max_timeout(wdd);
>
>         /*
>          * Note: now that all watchdog_device data has been verified, we
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index ef8edec..08b48bb 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -200,8 +200,7 @@ static int watchdog_set_timeout(struct watchdog_device *wddev,
>             !(wddev->info->options & WDIOF_SETTIMEOUT))
>                 return -EOPNOTSUPP;
>
> -       if ((wddev->max_timeout != 0) &&
> -           (timeout < wddev->min_timeout || timeout > wddev->max_timeout))
> +       if (watchdog_timeout_invalid(wddev, timeout))
>                 return -EINVAL;
>
>         mutex_lock(&wddev->lock);
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 3a9df2f..2a3038e 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -118,6 +118,13 @@ static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway
>                 set_bit(WDOG_NO_WAY_OUT, &wdd->status);
>  }
>
> +/* Use the following function to check if a timeout value is invalid */
> +static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t)
> +{
> +       return ((wdd->max_timeout != 0) &&
> +               (t < wdd->min_timeout || t > wdd->max_timeout));
> +}
> +
>  /* Use the following functions to manipulate watchdog driver specific data */
>  static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
>  {
> @@ -130,6 +137,8 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
>  }
>
>  /* drivers/watchdog/watchdog_core.c */
> +extern int watchdog_init_timeout(struct watchdog_device *wdd,
> +                                 unsigned int timeout_parm, struct device *dev);
>  extern int watchdog_register_device(struct watchdog_device *);
>  extern void watchdog_unregister_device(struct watchdog_device *);
>



--
Fabio Porcedda

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Wim Van Sebroeck]

Hi Fabio,

> > I must admit that the different iterations of this patch only made it better.
> > I like the idea that you are proposing here (put the default timeout in the
> > watchdog_device struct and use a helper function to set the timeout parameter
> > value or the timeout-sec dt value). To be able to use other mechanism in the
> > future also, I think it make more sense to pass the device instead of the
> > device_node.
> 
> If i understand correctly you want to use "struct platform_device"
> instead of "struct device_node",
> in the function watchdog_init_timeout?

No struct device instead of struct device_node.

> > I also detailed your documentation a bit more and incorporated above changes
> > in an adjusted patch. Can you have a look at it and if OK for you I will put
> > it in linux-watchdog-next as your v10 :-).
> 
> That's good for sure :)

Will add it to the linux-watchdog-next tree today.

Kind regards,
Wim.

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Wim Van Sebroeck]

Hi Fabio,

> > I must admit that the different iterations of this patch only made it better.
> > I like the idea that you are proposing here (put the default timeout in the
> > watchdog_device struct and use a helper function to set the timeout parameter
> > value or the timeout-sec dt value). To be able to use other mechanism in the
> > future also, I think it make more sense to pass the device instead of the
> > device_node.
> 
> If i understand correctly you want to use "struct platform_device"
> instead of "struct device_node",
> in the function watchdog_init_timeout?

No struct device instead of struct device_node.

> > I also detailed your documentation a bit more and incorporated above changes
> > in an adjusted patch. Can you have a look at it and if OK for you I will put
> > it in linux-watchdog-next as your v10 :-).
> 
> That's good for sure :)

Will add it to the linux-watchdog-next tree today.

Kind regards,
Wim.

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Wed, Feb 13, 2013 at 10:44 AM, Wim Van Sebroeck <wim-IQzOog9fTRqzQB+pC5nmwQ@public.gmane.org> wrote:
> Hi Fabio,
>
>> > I must admit that the different iterations of this patch only made it better.
>> > I like the idea that you are proposing here (put the default timeout in the
>> > watchdog_device struct and use a helper function to set the timeout parameter
>> > value or the timeout-sec dt value). To be able to use other mechanism in the
>> > future also, I think it make more sense to pass the device instead of the
>> > device_node.
>>
>> If i understand correctly you want to use "struct platform_device"
>> instead of "struct device_node",
>> in the function watchdog_init_timeout?
>
> No struct device instead of struct device_node.
>
>> > I also detailed your documentation a bit more and incorporated above changes
>> > in an adjusted patch. Can you have a look at it and if OK for you I will put
>> > it in linux-watchdog-next as your v10 :-).
>>
>> That's good for sure :)
>
> Will add it to the linux-watchdog-next tree today.
>

In a few minutes I will send the v8 patch set.

Best regards
--
Fabio Porcedda

> Kind regards,
> Wim.
>

Re: [PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Wed, Feb 13, 2013 at 10:44 AM, Wim Van Sebroeck <wim@iguana.be> wrote:
> Hi Fabio,
>
>> > I must admit that the different iterations of this patch only made it better.
>> > I like the idea that you are proposing here (put the default timeout in the
>> > watchdog_device struct and use a helper function to set the timeout parameter
>> > value or the timeout-sec dt value). To be able to use other mechanism in the
>> > future also, I think it make more sense to pass the device instead of the
>> > device_node.
>>
>> If i understand correctly you want to use "struct platform_device"
>> instead of "struct device_node",
>> in the function watchdog_init_timeout?
>
> No struct device instead of struct device_node.
>
>> > I also detailed your documentation a bit more and incorporated above changes
>> > in an adjusted patch. Can you have a look at it and if OK for you I will put
>> > it in linux-watchdog-next as your v10 :-).
>>
>> That's good for sure :)
>
> Will add it to the linux-watchdog-next tree today.
>

In a few minutes I will send the v8 patch set.

Best regards
--
Fabio Porcedda

> Kind regards,
> Wim.
>

[PATCH v7 1/6] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Wed, Feb 13, 2013 at 10:44 AM, Wim Van Sebroeck <wim@iguana.be> wrote:
> Hi Fabio,
>
>> > I must admit that the different iterations of this patch only made it better.
>> > I like the idea that you are proposing here (put the default timeout in the
>> > watchdog_device struct and use a helper function to set the timeout parameter
>> > value or the timeout-sec dt value). To be able to use other mechanism in the
>> > future also, I think it make more sense to pass the device instead of the
>> > device_node.
>>
>> If i understand correctly you want to use "struct platform_device"
>> instead of "struct device_node",
>> in the function watchdog_init_timeout?
>
> No struct device instead of struct device_node.
>
>> > I also detailed your documentation a bit more and incorporated above changes
>> > in an adjusted patch. Can you have a look at it and if OK for you I will put
>> > it in linux-watchdog-next as your v10 :-).
>>
>> That's good for sure :)
>
> Will add it to the linux-watchdog-next tree today.
>

In a few minutes I will send the v8 patch set.

Best regards
--
Fabio Porcedda

> Kind regards,
> Wim.
>