[RFC PATCH v3 0/4] watchdog: core: dt: add support for the timeout-sec dt property

[RFC PATCH v3 0/4] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

Hi all,
this patchset add the timeout-sec property to the watchdog core,
in the next revision i will add the s3c2410 driver.

changes:
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 (4):
  watchdog: core: dt: add support for the timeout-sec dt property
  watchdog: add timeout-sec property binding to some drivers
  watchdog: orion_wdt: litte cleanup
  watchdog: WatchDog Timer Driver Core: fix comment

 .../devicetree/bindings/watchdog/atmel-wdt.txt     |  4 ++++
 .../devicetree/bindings/watchdog/marvel.txt        |  5 ++++
 .../devicetree/bindings/watchdog/pnx4008-wdt.txt   |  4 ++++
 Documentation/watchdog/watchdog-kernel-api.txt     |  4 ++++
 drivers/watchdog/Kconfig                           |  1 +
 drivers/watchdog/at91sam9_wdt.c                    | 17 ++++++++++----
 drivers/watchdog/orion_wdt.c                       | 13 ++++-------
 drivers/watchdog/pnx4008_wdt.c                     |  7 +++---
 drivers/watchdog/watchdog_core.c                   | 27 ++++++++++++++++++++++
 include/linux/watchdog.h                           |  6 ++++-
 10 files changed, 71 insertions(+), 17 deletions(-)

-- 
1.7.11.3

[RFC PATCH v3 1/4] 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 |  4 ++++
 drivers/watchdog/watchdog_core.c               | 27 ++++++++++++++++++++++++++
 include/linux/watchdog.h                       |  4 ++++
 3 files changed, 35 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..2208db7 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -212,3 +212,7 @@ 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.
+
+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..646be48 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -143,6 +143,33 @@ void watchdog_unregister_device(struct watchdog_device *wdd)
 }
 EXPORT_SYMBOL_GPL(watchdog_unregister_device);
 
+/**
+ * watchdog_init_timeout() - initialize the timeout field
+ * @parm_timeout: 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_timeout, struct device_node *node)
+{
+	unsigned int t = 0;
+
+	if ((parm_timeout >= wdd->min_timeout) &&
+	    (parm_timeout <= wdd->max_timeout)) {
+		wdd->timeout = parm_timeout;
+		return;
+	}
+
+	if (!node)
+		return;
+
+	of_property_read_u32(node, "timeout-sec", &t);
+	if ((t >= wdd->min_timeout) && (t <= wdd->max_timeout))
+		wdd->timeout = t;
+}
+EXPORT_SYMBOL_GPL(watchdog_init_timeout);
+
 static int __init watchdog_init(void)
 {
 	int err;
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index da70f0f..d09dda7 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -11,6 +11,7 @@
 
 #include <linux/ioctl.h>
 #include <linux/types.h>
+#include <linux/of.h>
 
 #define	WATCHDOG_IOCTL_BASE	'W'
 
@@ -177,6 +178,9 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 /* drivers/watchdog/core/watchdog_core.c */
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
+extern void watchdog_init_timeout(struct watchdog_device *wdd,
+				  unsigned int parm_timeout,
+				  struct device_node *node);
 
 #endif	/* __KERNEL__ */
 
-- 
1.7.11.3

Re: [RFC PATCH v3 1/4] watchdog: core: dt: add support for the timeout-sec dt property

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:16 Fri 05 Oct     , Fabio Porcedda wrote:
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  Documentation/watchdog/watchdog-kernel-api.txt |  4 ++++
>  drivers/watchdog/watchdog_core.c               | 27 ++++++++++++++++++++++++++
>  include/linux/watchdog.h                       |  4 ++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
> index 086638f..2208db7 100644
> --- a/Documentation/watchdog/watchdog-kernel-api.txt
> +++ b/Documentation/watchdog/watchdog-kernel-api.txt
> @@ -212,3 +212,7 @@ 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.
> +
> +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..646be48 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -143,6 +143,33 @@ void watchdog_unregister_device(struct watchdog_device *wdd)
>  }
>  EXPORT_SYMBOL_GPL(watchdog_unregister_device);
>  
> +/**
> + * watchdog_init_timeout() - initialize the timeout field
> + * @parm_timeout: 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_timeout, struct device_node *node)
> +{
> +	unsigned int t = 0;
> +
> +	if ((parm_timeout >= wdd->min_timeout) &&
> +	    (parm_timeout <= wdd->max_timeout)) {
> +		wdd->timeout = parm_timeout;
> +		return;
> +	}
> +
> +	if (!node)
> +		return;
> +
> +	of_property_read_u32(node, "timeout-sec", &t);
why I bother to comment
*make this of generic* this is not watchdog specific other driver can use it

Best Regards,
J.

Re: [RFC PATCH v3 1/4] watchdog: core: dt: add support for the timeout-sec dt property

[Fabio Porcedda]

On Fri, Oct 5, 2012 at 1:16 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 12:16 Fri 05 Oct     , Fabio Porcedda wrote:
>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>  Documentation/watchdog/watchdog-kernel-api.txt |  4 ++++
>>  drivers/watchdog/watchdog_core.c               | 27 ++++++++++++++++++++++++++
>>  include/linux/watchdog.h                       |  4 ++++
>>  3 files changed, 35 insertions(+)
>>
>> diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
>> index 086638f..2208db7 100644
>> --- a/Documentation/watchdog/watchdog-kernel-api.txt
>> +++ b/Documentation/watchdog/watchdog-kernel-api.txt
>> @@ -212,3 +212,7 @@ 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.
>> +
>> +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..646be48 100644
>> --- a/drivers/watchdog/watchdog_core.c
>> +++ b/drivers/watchdog/watchdog_core.c
>> @@ -143,6 +143,33 @@ void watchdog_unregister_device(struct watchdog_device *wdd)
>>  }
>>  EXPORT_SYMBOL_GPL(watchdog_unregister_device);
>>
>> +/**
>> + * watchdog_init_timeout() - initialize the timeout field
>> + * @parm_timeout: 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_timeout, struct device_node *node)
>> +{
>> +     unsigned int t = 0;
>> +
>> +     if ((parm_timeout >= wdd->min_timeout) &&
>> +         (parm_timeout <= wdd->max_timeout)) {
>> +             wdd->timeout = parm_timeout;
>> +             return;
>> +     }
>> +
>> +     if (!node)
>> +             return;
>> +
>> +     of_property_read_u32(node, "timeout-sec", &t);
> why I bother to comment
> *make this of generic* this is not watchdog specific other driver can use it

You mean to add the function in of.h and use that function inside
watchdog_init_timeout or to
use the of_* function instead of the watchdog_init_timeout function?

Do you like the use of watchdog_init_timeout function inside the
at91sam9_wdt driver?

Best regards
-- 
Fabio Porcedda

[RFC PATCH v3 2/4] watchdog: add timeout-sec property binding to some drivers

[Fabio Porcedda]

The binding is provided by the watchdog core.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 .../devicetree/bindings/watchdog/atmel-wdt.txt          |  4 ++++
 Documentation/devicetree/bindings/watchdog/marvel.txt   |  5 +++++
 .../devicetree/bindings/watchdog/pnx4008-wdt.txt        |  4 ++++
 drivers/watchdog/Kconfig                                |  1 +
 drivers/watchdog/at91sam9_wdt.c                         | 17 +++++++++++++----
 drivers/watchdog/orion_wdt.c                            |  9 +++------
 drivers/watchdog/pnx4008_wdt.c                          |  7 +++----
 7 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
index 2957ebb..27af70d 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/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/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt b/Documentation/devicetree/bindings/watchdog/pnx4008-wdt.txt
index 7c7f688..47461f5 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/Kconfig b/drivers/watchdog/Kconfig
index ad1bb93..dda695f 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -114,6 +114,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..f901cc0 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -32,7 +32,6 @@
 #include <linux/timer.h>
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
-#include <linux/of.h>
 
 #include "at91sam9_wdt.h"
 
@@ -57,8 +56,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 = 0;
 module_param(heartbeat, int, 0);
 MODULE_PARM_DESC(heartbeat, "Watchdog heartbeats in seconds. "
 	"(default = " __MODULE_STRING(WDT_HEARTBEAT) ")");
@@ -255,6 +256,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 +280,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 +291,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;
 }
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c20f96b..3417f1b 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -24,7 +24,6 @@
 #include <linux/spinlock.h>
 #include <linux/clk.h>
 #include <linux/err.h>
-#include <linux/of.h>
 #include <mach/bridge-regs.h>
 
 /*
@@ -162,12 +161,10 @@ static int __devinit 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.max_timeout = wdt_max_duration;
+	orion_wdt.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);
@@ -177,7 +174,7 @@ static int __devinit 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;
 }
 
diff --git a/drivers/watchdog/pnx4008_wdt.c b/drivers/watchdog/pnx4008_wdt.c
index 87722e1..152e9fb 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 __devinit 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 __devinit 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 __devinit 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.7.11.3

[RFC PATCH v3 3/4] watchdog: orion_wdt: litte cleanup

[Fabio Porcedda]

- Move the min_timeout initialization inside the orion_wdt definition.
- Change orion_wdt_of_match_table from __devinitdata to __devinitconst.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 drivers/watchdog/orion_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index 3417f1b..9e09598 100644
--- a/drivers/watchdog/orion_wdt.c
+++ b/drivers/watchdog/orion_wdt.c
@@ -139,6 +139,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 __devinit orion_wdt_probe(struct platform_device *pdev)
@@ -161,7 +162,6 @@ static int __devinit orion_wdt_probe(struct platform_device *pdev)
 
 	wdt_max_duration = WDT_MAX_CYCLE_COUNT / wdt_tclk;
 
-	orion_wdt.min_timeout = 1;
 	orion_wdt.max_timeout = wdt_max_duration;
 	orion_wdt.timeout = wdt_max_duration;
 	watchdog_init_timeout(&orion_wdt, heartbeat, pdev->dev.of_node);
@@ -190,7 +190,7 @@ static void orion_wdt_shutdown(struct platform_device *pdev)
 	orion_wdt_stop(&orion_wdt);
 }
 
-static const struct of_device_id orion_wdt_of_match_table[] __devinitdata = {
+static const struct of_device_id orion_wdt_of_match_table[] __devinitconst = {
 	{ .compatible = "marvell,orion-wdt", },
 	{},
 };
-- 
1.7.11.3

[RFC PATCH v3 4/4] watchdog: WatchDog Timer Driver Core: fix comment

[Fabio Porcedda]

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 include/linux/watchdog.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index d09dda7..9e6a8f2 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -175,7 +175,7 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 	return wdd->driver_data;
 }
 
-/* drivers/watchdog/core/watchdog_core.c */
+/* drivers/watchdog/watchdog_core.c */
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 extern void watchdog_init_timeout(struct watchdog_device *wdd,
-- 
1.7.11.3