[PATCH v6 0/5] watchdog: orion_wdt & at91sam9_wdt: improve dt support

[PATCH v6 0/5] watchdog: orion_wdt & at91sam9_wdt: improve dt support

[Fabio Porcedda]

Hi all,

This set of patches is for adding device tree support to the
at91sam9_wdt driver and to add the new timeout property to
the watchdog core.

I've tested the at91sam9_wdt modifications on an at91sam9260 board (evk-pro3).

Changes:
v6:
 - add watchdog core proprty timeout binding
 - add as example to the orion_wdt driver the new timeout binding
v5:
 - split dts commit in two, one for socs and one for boards.
v4:
 - use "atmel,at91sam9260-wdt" as compatible
v3:
 - add heartbeat option
 - default disabled for all the soc, enable only for evk-pro3 board
 - add at91sam9263 and at91sam9g45 soc
v2:
 - add missing to and cc addresses



Fabio Porcedda (5):
  watchdog: core: dt: add support for the timeout device tree property
  watchdog: orion_wdt: dt: add the timeout property binding
  watchdog: at91sam9_wdt: add device tree support
  ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45
  ARM: at91/dts: evk-pro3: enable watchdog

 .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
 .../devicetree/bindings/watchdog/marvel.txt         |  5 +++++
 Documentation/watchdog/watchdog-kernel-api.txt      |  3 +++
 arch/arm/boot/dts/at91sam9260.dtsi                  |  6 ++++++
 arch/arm/boot/dts/at91sam9263.dtsi                  |  6 ++++++
 arch/arm/boot/dts/at91sam9g45.dtsi                  |  6 ++++++
 arch/arm/boot/dts/evk-pro3.dts                      |  4 ++++
 drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
 drivers/watchdog/orion_wdt.c                        |  2 +-
 include/linux/watchdog.h                            | 11 +++++++++++
 10 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt

-- 
1.7.11.3

[PATCH v6 0/5] watchdog: orion_wdt & at91sam9_wdt: improve dt support

[Fabio Porcedda]

Hi all,

This set of patches is for adding device tree support to the
at91sam9_wdt driver and to add the new timeout property to
the watchdog core.

I've tested the at91sam9_wdt modifications on an at91sam9260 board (evk-pro3).

Changes:
v6:
 - add watchdog core proprty timeout binding
 - add as example to the orion_wdt driver the new timeout binding
v5:
 - split dts commit in two, one for socs and one for boards.
v4:
 - use "atmel,at91sam9260-wdt" as compatible
v3:
 - add heartbeat option
 - default disabled for all the soc, enable only for evk-pro3 board
 - add at91sam9263 and at91sam9g45 soc
v2:
 - add missing to and cc addresses



Fabio Porcedda (5):
  watchdog: core: dt: add support for the timeout device tree property
  watchdog: orion_wdt: dt: add the timeout property binding
  watchdog: at91sam9_wdt: add device tree support
  ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45
  ARM: at91/dts: evk-pro3: enable watchdog

 .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
 .../devicetree/bindings/watchdog/marvel.txt         |  5 +++++
 Documentation/watchdog/watchdog-kernel-api.txt      |  3 +++
 arch/arm/boot/dts/at91sam9260.dtsi                  |  6 ++++++
 arch/arm/boot/dts/at91sam9263.dtsi                  |  6 ++++++
 arch/arm/boot/dts/at91sam9g45.dtsi                  |  6 ++++++
 arch/arm/boot/dts/evk-pro3.dts                      |  4 ++++
 drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
 drivers/watchdog/orion_wdt.c                        |  2 +-
 include/linux/watchdog.h                            | 11 +++++++++++
 10 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt

-- 
1.7.11.3

[PATCH v6 0/5] watchdog: orion_wdt & at91sam9_wdt: improve dt support

[Fabio Porcedda]

Hi all,

This set of patches is for adding device tree support to the
at91sam9_wdt driver and to add the new timeout property to
the watchdog core.

I've tested the at91sam9_wdt modifications on an at91sam9260 board (evk-pro3).

Changes:
v6:
 - add watchdog core proprty timeout binding
 - add as example to the orion_wdt driver the new timeout binding
v5:
 - split dts commit in two, one for socs and one for boards.
v4:
 - use "atmel,at91sam9260-wdt" as compatible
v3:
 - add heartbeat option
 - default disabled for all the soc, enable only for evk-pro3 board
 - add at91sam9263 and at91sam9g45 soc
v2:
 - add missing to and cc addresses



Fabio Porcedda (5):
  watchdog: core: dt: add support for the timeout device tree property
  watchdog: orion_wdt: dt: add the timeout property binding
  watchdog: at91sam9_wdt: add device tree support
  ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45
  ARM: at91/dts: evk-pro3: enable watchdog

 .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
 .../devicetree/bindings/watchdog/marvel.txt         |  5 +++++
 Documentation/watchdog/watchdog-kernel-api.txt      |  3 +++
 arch/arm/boot/dts/at91sam9260.dtsi                  |  6 ++++++
 arch/arm/boot/dts/at91sam9263.dtsi                  |  6 ++++++
 arch/arm/boot/dts/at91sam9g45.dtsi                  |  6 ++++++
 arch/arm/boot/dts/evk-pro3.dts                      |  4 ++++
 drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
 drivers/watchdog/orion_wdt.c                        |  2 +-
 include/linux/watchdog.h                            | 11 +++++++++++
 10 files changed, 82 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt

-- 
1.7.11.3

[PATCH v6 1/5] watchdog: core: dt: add support for the timeout device tree property

[Fabio Porcedda]

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/watchdog/watchdog-kernel-api.txt |  3 +++
 include/linux/watchdog.h                       | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..decd981 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -212,3 +212,6 @@ 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_probe_dt function allows you to retrieve the timeout property
+from the device tree.
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index da70f0f..06f9384 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'
 
@@ -174,6 +175,16 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 	return wdd->driver_data;
 }
 
+/* Use the following function to retrieve the timeout property from dt */
+static inline void watchdog_probe_dt(struct watchdog_device *wdd,
+				     struct device_node *node)
+{
+	if (!node)
+		return;
+
+	of_property_read_u32(node, "timeout", &wdd->timeout);
+}
+
 /* drivers/watchdog/core/watchdog_core.c */
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
-- 
1.7.11.3

[PATCH v6 1/5] watchdog: core: dt: add support for the timeout device tree property

[Fabio Porcedda]

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Documentation/watchdog/watchdog-kernel-api.txt |  3 +++
 include/linux/watchdog.h                       | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..decd981 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -212,3 +212,6 @@ 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_probe_dt function allows you to retrieve the timeout property
+from the device tree.
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index da70f0f..06f9384 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'
 
@@ -174,6 +175,16 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 	return wdd->driver_data;
 }
 
+/* Use the following function to retrieve the timeout property from dt */
+static inline void watchdog_probe_dt(struct watchdog_device *wdd,
+				     struct device_node *node)
+{
+	if (!node)
+		return;
+
+	of_property_read_u32(node, "timeout", &wdd->timeout);
+}
+
 /* drivers/watchdog/core/watchdog_core.c */
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
-- 
1.7.11.3

[PATCH v6 1/5] watchdog: core: dt: add support for the timeout device tree property

[Fabio Porcedda]

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Documentation/watchdog/watchdog-kernel-api.txt |  3 +++
 include/linux/watchdog.h                       | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 086638f..decd981 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -212,3 +212,6 @@ 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_probe_dt function allows you to retrieve the timeout property
+from the device tree.
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index da70f0f..06f9384 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'
 
@@ -174,6 +175,16 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 	return wdd->driver_data;
 }
 
+/* Use the following function to retrieve the timeout property from dt */
+static inline void watchdog_probe_dt(struct watchdog_device *wdd,
+				     struct device_node *node)
+{
+	if (!node)
+		return;
+
+	of_property_read_u32(node, "timeout", &wdd->timeout);
+}
+
 /* drivers/watchdog/core/watchdog_core.c */
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
-- 
1.7.11.3

[PATCH v6 2/5] watchdog: orion_wdt: dt: add the timeout property binding

[Fabio Porcedda]

The binding is provided by the watchdog core.

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/devicetree/bindings/watchdog/marvel.txt | 5 +++++
 drivers/watchdog/orion_wdt.c                          | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt
index 0b2503a..66c2b8c 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	: Contains the watchdog timeout in seconds
+
 Example:
 
 	wdt@20300 {
 		compatible = "marvell,orion-wdt";
 		reg = <0x20300 0x28>;
+		timeout = <10>;
 		status = "okay";
 	};
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c20f96b..3dc7481 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>
 
 /*
@@ -168,6 +167,7 @@ static int __devinit orion_wdt_probe(struct platform_device *pdev)
 	orion_wdt.timeout = heartbeat;
 	orion_wdt.min_timeout = 1;
 	orion_wdt.max_timeout = wdt_max_duration;
+	watchdog_probe_dt(&orion_wdt, pdev->dev.of_node);
 
 	watchdog_set_nowayout(&orion_wdt, nowayout);
 	ret = watchdog_register_device(&orion_wdt);
-- 
1.7.11.3

[PATCH v6 2/5] watchdog: orion_wdt: dt: add the timeout property binding

[Fabio Porcedda]

The binding is provided by the watchdog core.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Documentation/devicetree/bindings/watchdog/marvel.txt | 5 +++++
 drivers/watchdog/orion_wdt.c                          | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt
index 0b2503a..66c2b8c 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	: Contains the watchdog timeout in seconds
+
 Example:
 
 	wdt@20300 {
 		compatible = "marvell,orion-wdt";
 		reg = <0x20300 0x28>;
+		timeout = <10>;
 		status = "okay";
 	};
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c20f96b..3dc7481 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>
 
 /*
@@ -168,6 +167,7 @@ static int __devinit orion_wdt_probe(struct platform_device *pdev)
 	orion_wdt.timeout = heartbeat;
 	orion_wdt.min_timeout = 1;
 	orion_wdt.max_timeout = wdt_max_duration;
+	watchdog_probe_dt(&orion_wdt, pdev->dev.of_node);
 
 	watchdog_set_nowayout(&orion_wdt, nowayout);
 	ret = watchdog_register_device(&orion_wdt);
-- 
1.7.11.3

[PATCH v6 2/5] watchdog: orion_wdt: dt: add the timeout property binding

[Fabio Porcedda]

The binding is provided by the watchdog core.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 Documentation/devicetree/bindings/watchdog/marvel.txt | 5 +++++
 drivers/watchdog/orion_wdt.c                          | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/watchdog/marvel.txt b/Documentation/devicetree/bindings/watchdog/marvel.txt
index 0b2503a..66c2b8c 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	: Contains the watchdog timeout in seconds
+
 Example:
 
 	wdt at 20300 {
 		compatible = "marvell,orion-wdt";
 		reg = <0x20300 0x28>;
+		timeout = <10>;
 		status = "okay";
 	};
diff --git a/drivers/watchdog/orion_wdt.c b/drivers/watchdog/orion_wdt.c
index c20f96b..3dc7481 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>
 
 /*
@@ -168,6 +167,7 @@ static int __devinit orion_wdt_probe(struct platform_device *pdev)
 	orion_wdt.timeout = heartbeat;
 	orion_wdt.min_timeout = 1;
 	orion_wdt.max_timeout = wdt_max_duration;
+	watchdog_probe_dt(&orion_wdt, pdev->dev.of_node);
 
 	watchdog_set_nowayout(&orion_wdt, nowayout);
 	ret = watchdog_register_device(&orion_wdt);
-- 
1.7.11.3

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

Tested on an at91sam9260 board (evk-pro3)

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
 drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
new file mode 100644
index 0000000..65c1755
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
@@ -0,0 +1,19 @@
+* Atmel Watchdog Timers
+
+** at91sam9-wdt
+
+Required properties:
+- compatible: must be "atmel,at91sam9260-wdt".
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+Optional properties:
+- timeout: contains the watchdog timeout in seconds.
+
+Example:
+
+	watchdog@fffffd40 {
+		compatible = "atmel,at91sam9260-wdt";
+		reg = <0xfffffd40 0x10>;
+		timeout = <10>;
+	};
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index 05e1be8..c9e6bfa 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -32,6 +32,7 @@
 #include <linux/timer.h>
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
+#include <linux/of.h>
 
 #include "at91sam9_wdt.h"
 
@@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
 	.fops		= &at91wdt_fops,
 };
 
+static inline void __init at91wdt_probe_dt(struct device_node *node)
+{
+	if (!node)
+		return;
+
+	of_property_read_u32(node, "timeout", &heartbeat);
+}
+
 static int __init at91wdt_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
@@ -272,6 +281,8 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	at91wdt_probe_dt(pdev->dev.of_node);
+
 	/* Set watchdog */
 	res = at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
 	if (res)
@@ -302,11 +313,21 @@ static int __exit at91wdt_remove(struct platform_device *pdev)
 	return res;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id at91_wdt_dt_ids[] = {
+	{ .compatible = "atmel,at91sam9260-wdt" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, at91_wdt_dt_ids);
+#endif
+
 static struct platform_driver at91wdt_driver = {
 	.remove		= __exit_p(at91wdt_remove),
 	.driver		= {
 		.name	= "at91_wdt",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(at91_wdt_dt_ids),
 	},
 };
 
-- 
1.7.11.3

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

Tested on an at91sam9260 board (evk-pro3)

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
 drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
new file mode 100644
index 0000000..65c1755
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
@@ -0,0 +1,19 @@
+* Atmel Watchdog Timers
+
+** at91sam9-wdt
+
+Required properties:
+- compatible: must be "atmel,at91sam9260-wdt".
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+Optional properties:
+- timeout: contains the watchdog timeout in seconds.
+
+Example:
+
+	watchdog@fffffd40 {
+		compatible = "atmel,at91sam9260-wdt";
+		reg = <0xfffffd40 0x10>;
+		timeout = <10>;
+	};
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index 05e1be8..c9e6bfa 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -32,6 +32,7 @@
 #include <linux/timer.h>
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
+#include <linux/of.h>
 
 #include "at91sam9_wdt.h"
 
@@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
 	.fops		= &at91wdt_fops,
 };
 
+static inline void __init at91wdt_probe_dt(struct device_node *node)
+{
+	if (!node)
+		return;
+
+	of_property_read_u32(node, "timeout", &heartbeat);
+}
+
 static int __init at91wdt_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
@@ -272,6 +281,8 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	at91wdt_probe_dt(pdev->dev.of_node);
+
 	/* Set watchdog */
 	res = at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
 	if (res)
@@ -302,11 +313,21 @@ static int __exit at91wdt_remove(struct platform_device *pdev)
 	return res;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id at91_wdt_dt_ids[] = {
+	{ .compatible = "atmel,at91sam9260-wdt" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, at91_wdt_dt_ids);
+#endif
+
 static struct platform_driver at91wdt_driver = {
 	.remove		= __exit_p(at91wdt_remove),
 	.driver		= {
 		.name	= "at91_wdt",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(at91_wdt_dt_ids),
 	},
 };
 
-- 
1.7.11.3

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

Tested on an at91sam9260 board (evk-pro3)

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
 drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
new file mode 100644
index 0000000..65c1755
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
@@ -0,0 +1,19 @@
+* Atmel Watchdog Timers
+
+** at91sam9-wdt
+
+Required properties:
+- compatible: must be "atmel,at91sam9260-wdt".
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+Optional properties:
+- timeout: contains the watchdog timeout in seconds.
+
+Example:
+
+	watchdog at fffffd40 {
+		compatible = "atmel,at91sam9260-wdt";
+		reg = <0xfffffd40 0x10>;
+		timeout = <10>;
+	};
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index 05e1be8..c9e6bfa 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -32,6 +32,7 @@
 #include <linux/timer.h>
 #include <linux/bitops.h>
 #include <linux/uaccess.h>
+#include <linux/of.h>
 
 #include "at91sam9_wdt.h"
 
@@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
 	.fops		= &at91wdt_fops,
 };
 
+static inline void __init at91wdt_probe_dt(struct device_node *node)
+{
+	if (!node)
+		return;
+
+	of_property_read_u32(node, "timeout", &heartbeat);
+}
+
 static int __init at91wdt_probe(struct platform_device *pdev)
 {
 	struct resource	*r;
@@ -272,6 +281,8 @@ static int __init at91wdt_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	at91wdt_probe_dt(pdev->dev.of_node);
+
 	/* Set watchdog */
 	res = at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
 	if (res)
@@ -302,11 +313,21 @@ static int __exit at91wdt_remove(struct platform_device *pdev)
 	return res;
 }
 
+#if defined(CONFIG_OF)
+static const struct of_device_id at91_wdt_dt_ids[] = {
+	{ .compatible = "atmel,at91sam9260-wdt" },
+	{ /* sentinel */ }
+};
+
+MODULE_DEVICE_TABLE(of, at91_wdt_dt_ids);
+#endif
+
 static struct platform_driver at91wdt_driver = {
 	.remove		= __exit_p(at91wdt_remove),
 	.driver		= {
 		.name	= "at91_wdt",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(at91_wdt_dt_ids),
 	},
 };
 
-- 
1.7.11.3

[PATCH v6 4/5] ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45

[Fabio Porcedda]

Tested on an at91sam9260 board (evk-pro3).

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/at91sam9260.dtsi | 6 ++++++
 arch/arm/boot/dts/at91sam9263.dtsi | 6 ++++++
 arch/arm/boot/dts/at91sam9g45.dtsi | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index 7c95f76..7612193 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -236,6 +236,12 @@
 					trigger-external;
 				};
 			};
+
+			watchdog@fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand@40000000 {
diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
index 195019b..d822240 100644
--- a/arch/arm/boot/dts/at91sam9263.dtsi
+++ b/arch/arm/boot/dts/at91sam9263.dtsi
@@ -185,6 +185,12 @@
 				interrupts = <24 4 2>;
 				status = "disabled";
 			};
+
+			watchdog@fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand@40000000 {
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 63751b1..df998ab 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -242,6 +242,12 @@
 					trigger-value = <0x6>;
 				};
 			};
+
+			watchdog@fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand@40000000 {
-- 
1.7.11.3

[PATCH v6 4/5] ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45

[Fabio Porcedda]

Tested on an at91sam9260 board (evk-pro3).

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 arch/arm/boot/dts/at91sam9260.dtsi | 6 ++++++
 arch/arm/boot/dts/at91sam9263.dtsi | 6 ++++++
 arch/arm/boot/dts/at91sam9g45.dtsi | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index 7c95f76..7612193 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -236,6 +236,12 @@
 					trigger-external;
 				};
 			};
+
+			watchdog@fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand@40000000 {
diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
index 195019b..d822240 100644
--- a/arch/arm/boot/dts/at91sam9263.dtsi
+++ b/arch/arm/boot/dts/at91sam9263.dtsi
@@ -185,6 +185,12 @@
 				interrupts = <24 4 2>;
 				status = "disabled";
 			};
+
+			watchdog@fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand@40000000 {
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 63751b1..df998ab 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -242,6 +242,12 @@
 					trigger-value = <0x6>;
 				};
 			};
+
+			watchdog@fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand@40000000 {
-- 
1.7.11.3

[PATCH v6 4/5] ARM: at91/dts: add at91sam9_wdt driver to at91sam926x, at91sam9g45

[Fabio Porcedda]

Tested on an at91sam9260 board (evk-pro3).

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 arch/arm/boot/dts/at91sam9260.dtsi | 6 ++++++
 arch/arm/boot/dts/at91sam9263.dtsi | 6 ++++++
 arch/arm/boot/dts/at91sam9g45.dtsi | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index 7c95f76..7612193 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -236,6 +236,12 @@
 					trigger-external;
 				};
 			};
+
+			watchdog at fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand at 40000000 {
diff --git a/arch/arm/boot/dts/at91sam9263.dtsi b/arch/arm/boot/dts/at91sam9263.dtsi
index 195019b..d822240 100644
--- a/arch/arm/boot/dts/at91sam9263.dtsi
+++ b/arch/arm/boot/dts/at91sam9263.dtsi
@@ -185,6 +185,12 @@
 				interrupts = <24 4 2>;
 				status = "disabled";
 			};
+
+			watchdog at fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand at 40000000 {
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 63751b1..df998ab 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -242,6 +242,12 @@
 					trigger-value = <0x6>;
 				};
 			};
+
+			watchdog at fffffd40 {
+				compatible = "atmel,at91sam9260-wdt";
+				reg = <0xfffffd40 0x10>;
+				status = "disabled";
+			};
 		};
 
 		nand0: nand at 40000000 {
-- 
1.7.11.3

[PATCH v6 5/5] ARM: at91/dts: evk-pro3: enable watchdog

[Fabio Porcedda]

Tested on evk-pro3.

Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 arch/arm/boot/dts/evk-pro3.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/evk-pro3.dts b/arch/arm/boot/dts/evk-pro3.dts
index b7354e6..ce959ee 100644
--- a/arch/arm/boot/dts/evk-pro3.dts
+++ b/arch/arm/boot/dts/evk-pro3.dts
@@ -26,6 +26,10 @@
 				atmel,vbus-gpio = <&pioC 5 0>;
 				status = "okay";
 			};
+
+			watchdog@fffffd40 {
+				status = "okay";
+			};
 		};
 
 		usb0: ohci@00500000 {
-- 
1.7.11.3

[PATCH v6 5/5] ARM: at91/dts: evk-pro3: enable watchdog

[Fabio Porcedda]

Tested on evk-pro3.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 arch/arm/boot/dts/evk-pro3.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/evk-pro3.dts b/arch/arm/boot/dts/evk-pro3.dts
index b7354e6..ce959ee 100644
--- a/arch/arm/boot/dts/evk-pro3.dts
+++ b/arch/arm/boot/dts/evk-pro3.dts
@@ -26,6 +26,10 @@
 				atmel,vbus-gpio = <&pioC 5 0>;
 				status = "okay";
 			};
+
+			watchdog@fffffd40 {
+				status = "okay";
+			};
 		};
 
 		usb0: ohci@00500000 {
-- 
1.7.11.3

[PATCH v6 5/5] ARM: at91/dts: evk-pro3: enable watchdog

[Fabio Porcedda]

Tested on evk-pro3.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 arch/arm/boot/dts/evk-pro3.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/evk-pro3.dts b/arch/arm/boot/dts/evk-pro3.dts
index b7354e6..ce959ee 100644
--- a/arch/arm/boot/dts/evk-pro3.dts
+++ b/arch/arm/boot/dts/evk-pro3.dts
@@ -26,6 +26,10 @@
 				atmel,vbus-gpio = <&pioC 5 0>;
 				status = "okay";
 			};
+
+			watchdog at fffffd40 {
+				status = "okay";
+			};
 		};
 
 		usb0: ohci at 00500000 {
-- 
1.7.11.3

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> Tested on an at91sam9260 board (evk-pro3)
> 
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> new file mode 100644
> index 0000000..65c1755
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> @@ -0,0 +1,19 @@
> +* Atmel Watchdog Timers
> +
> +** at91sam9-wdt
> +
> +Required properties:
> +- compatible: must be "atmel,at91sam9260-wdt".
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +Optional properties:
> +- timeout: contains the watchdog timeout in seconds.
> +
> +Example:
> +
> +	watchdog@fffffd40 {
> +		compatible = "atmel,at91sam9260-wdt";
> +		reg = <0xfffffd40 0x10>;
> +		timeout = <10>;
> +	};
> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> index 05e1be8..c9e6bfa 100644
> --- a/drivers/watchdog/at91sam9_wdt.c
> +++ b/drivers/watchdog/at91sam9_wdt.c
> @@ -32,6 +32,7 @@
>  #include <linux/timer.h>
>  #include <linux/bitops.h>
>  #include <linux/uaccess.h>
> +#include <linux/of.h>
>  
>  #include "at91sam9_wdt.h"
>  
> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>  	.fops		= &at91wdt_fops,
>  };
>  
> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> +{
> +	if (!node)
> +		return;
> +
> +	of_property_read_u32(node, "timeout", &heartbeat);
> +}
> +

In patch #1 you add a function to do this, and then you don't make use
of it here ?

Or am i missing something?

   Thanks
	Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> Tested on an at91sam9260 board (evk-pro3)
> 
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> new file mode 100644
> index 0000000..65c1755
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> @@ -0,0 +1,19 @@
> +* Atmel Watchdog Timers
> +
> +** at91sam9-wdt
> +
> +Required properties:
> +- compatible: must be "atmel,at91sam9260-wdt".
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +Optional properties:
> +- timeout: contains the watchdog timeout in seconds.
> +
> +Example:
> +
> +	watchdog@fffffd40 {
> +		compatible = "atmel,at91sam9260-wdt";
> +		reg = <0xfffffd40 0x10>;
> +		timeout = <10>;
> +	};
> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> index 05e1be8..c9e6bfa 100644
> --- a/drivers/watchdog/at91sam9_wdt.c
> +++ b/drivers/watchdog/at91sam9_wdt.c
> @@ -32,6 +32,7 @@
>  #include <linux/timer.h>
>  #include <linux/bitops.h>
>  #include <linux/uaccess.h>
> +#include <linux/of.h>
>  
>  #include "at91sam9_wdt.h"
>  
> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>  	.fops		= &at91wdt_fops,
>  };
>  
> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> +{
> +	if (!node)
> +		return;
> +
> +	of_property_read_u32(node, "timeout", &heartbeat);
> +}
> +

In patch #1 you add a function to do this, and then you don't make use
of it here ?

Or am i missing something?

   Thanks
	Andrew

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> Tested on an at91sam9260 board (evk-pro3)
> 
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> new file mode 100644
> index 0000000..65c1755
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> @@ -0,0 +1,19 @@
> +* Atmel Watchdog Timers
> +
> +** at91sam9-wdt
> +
> +Required properties:
> +- compatible: must be "atmel,at91sam9260-wdt".
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +Optional properties:
> +- timeout: contains the watchdog timeout in seconds.
> +
> +Example:
> +
> +	watchdog at fffffd40 {
> +		compatible = "atmel,at91sam9260-wdt";
> +		reg = <0xfffffd40 0x10>;
> +		timeout = <10>;
> +	};
> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> index 05e1be8..c9e6bfa 100644
> --- a/drivers/watchdog/at91sam9_wdt.c
> +++ b/drivers/watchdog/at91sam9_wdt.c
> @@ -32,6 +32,7 @@
>  #include <linux/timer.h>
>  #include <linux/bitops.h>
>  #include <linux/uaccess.h>
> +#include <linux/of.h>
>  
>  #include "at91sam9_wdt.h"
>  
> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>  	.fops		= &at91wdt_fops,
>  };
>  
> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> +{
> +	if (!node)
> +		return;
> +
> +	of_property_read_u32(node, "timeout", &heartbeat);
> +}
> +

In patch #1 you add a function to do this, and then you don't make use
of it here ?

Or am i missing something?

   Thanks
	Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> Tested on an at91sam9260 board (evk-pro3)
>>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>>  2 files changed, 40 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> new file mode 100644
>> index 0000000..65c1755
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> @@ -0,0 +1,19 @@
>> +* Atmel Watchdog Timers
>> +
>> +** at91sam9-wdt
>> +
>> +Required properties:
>> +- compatible: must be "atmel,at91sam9260-wdt".
>> +- reg: physical base address of the controller and length of memory mapped
>> +  region.
>> +
>> +Optional properties:
>> +- timeout: contains the watchdog timeout in seconds.
>> +
>> +Example:
>> +
>> +     watchdog@fffffd40 {
>> +             compatible = "atmel,at91sam9260-wdt";
>> +             reg = <0xfffffd40 0x10>;
>> +             timeout = <10>;
>> +     };
>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>> index 05e1be8..c9e6bfa 100644
>> --- a/drivers/watchdog/at91sam9_wdt.c
>> +++ b/drivers/watchdog/at91sam9_wdt.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/timer.h>
>>  #include <linux/bitops.h>
>>  #include <linux/uaccess.h>
>> +#include <linux/of.h>
>>
>>  #include "at91sam9_wdt.h"
>>
>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>>       .fops           = &at91wdt_fops,
>>  };
>>
>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>> +{
>> +     if (!node)
>> +             return;
>> +
>> +     of_property_read_u32(node, "timeout", &heartbeat);
>> +}
>> +
>
> In patch #1 you add a function to do this, and then you don't make use
> of it here ?
>
> Or am i missing something?

I'm using it on the patch #2 for the orion_wdt driver.
Do you think it's better to join the #1 and the #2 patch?

Best regards
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> Tested on an at91sam9260 board (evk-pro3)
>>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>>  2 files changed, 40 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> new file mode 100644
>> index 0000000..65c1755
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> @@ -0,0 +1,19 @@
>> +* Atmel Watchdog Timers
>> +
>> +** at91sam9-wdt
>> +
>> +Required properties:
>> +- compatible: must be "atmel,at91sam9260-wdt".
>> +- reg: physical base address of the controller and length of memory mapped
>> +  region.
>> +
>> +Optional properties:
>> +- timeout: contains the watchdog timeout in seconds.
>> +
>> +Example:
>> +
>> +     watchdog@fffffd40 {
>> +             compatible = "atmel,at91sam9260-wdt";
>> +             reg = <0xfffffd40 0x10>;
>> +             timeout = <10>;
>> +     };
>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>> index 05e1be8..c9e6bfa 100644
>> --- a/drivers/watchdog/at91sam9_wdt.c
>> +++ b/drivers/watchdog/at91sam9_wdt.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/timer.h>
>>  #include <linux/bitops.h>
>>  #include <linux/uaccess.h>
>> +#include <linux/of.h>
>>
>>  #include "at91sam9_wdt.h"
>>
>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>>       .fops           = &at91wdt_fops,
>>  };
>>
>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>> +{
>> +     if (!node)
>> +             return;
>> +
>> +     of_property_read_u32(node, "timeout", &heartbeat);
>> +}
>> +
>
> In patch #1 you add a function to do this, and then you don't make use
> of it here ?
>
> Or am i missing something?

I'm using it on the patch #2 for the orion_wdt driver.
Do you think it's better to join the #1 and the #2 patch?

Best regards
-- 
Fabio Porcedda

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> Tested on an at91sam9260 board (evk-pro3)
>>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>>  2 files changed, 40 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> new file mode 100644
>> index 0000000..65c1755
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> @@ -0,0 +1,19 @@
>> +* Atmel Watchdog Timers
>> +
>> +** at91sam9-wdt
>> +
>> +Required properties:
>> +- compatible: must be "atmel,at91sam9260-wdt".
>> +- reg: physical base address of the controller and length of memory mapped
>> +  region.
>> +
>> +Optional properties:
>> +- timeout: contains the watchdog timeout in seconds.
>> +
>> +Example:
>> +
>> +     watchdog at fffffd40 {
>> +             compatible = "atmel,at91sam9260-wdt";
>> +             reg = <0xfffffd40 0x10>;
>> +             timeout = <10>;
>> +     };
>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>> index 05e1be8..c9e6bfa 100644
>> --- a/drivers/watchdog/at91sam9_wdt.c
>> +++ b/drivers/watchdog/at91sam9_wdt.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/timer.h>
>>  #include <linux/bitops.h>
>>  #include <linux/uaccess.h>
>> +#include <linux/of.h>
>>
>>  #include "at91sam9_wdt.h"
>>
>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>>       .fops           = &at91wdt_fops,
>>  };
>>
>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>> +{
>> +     if (!node)
>> +             return;
>> +
>> +     of_property_read_u32(node, "timeout", &heartbeat);
>> +}
>> +
>
> In patch #1 you add a function to do this, and then you don't make use
> of it here ?
>
> Or am i missing something?

I'm using it on the patch #2 for the orion_wdt driver.
Do you think it's better to join the #1 and the #2 patch?

Best regards
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>>> Tested on an at91sam9260 board (evk-pro3)
>>>
>>> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>>>  2 files changed, 40 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>> new file mode 100644
>>> index 0000000..65c1755
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>> @@ -0,0 +1,19 @@
>>> +* Atmel Watchdog Timers
>>> +
>>> +** at91sam9-wdt
>>> +
>>> +Required properties:
>>> +- compatible: must be "atmel,at91sam9260-wdt".
>>> +- reg: physical base address of the controller and length of memory mapped
>>> +  region.
>>> +
>>> +Optional properties:
>>> +- timeout: contains the watchdog timeout in seconds.
>>> +
>>> +Example:
>>> +
>>> +     watchdog@fffffd40 {
>>> +             compatible = "atmel,at91sam9260-wdt";
>>> +             reg = <0xfffffd40 0x10>;
>>> +             timeout = <10>;
>>> +     };
>>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>>> index 05e1be8..c9e6bfa 100644
>>> --- a/drivers/watchdog/at91sam9_wdt.c
>>> +++ b/drivers/watchdog/at91sam9_wdt.c
>>> @@ -32,6 +32,7 @@
>>>  #include <linux/timer.h>
>>>  #include <linux/bitops.h>
>>>  #include <linux/uaccess.h>
>>> +#include <linux/of.h>
>>>
>>>  #include "at91sam9_wdt.h"
>>>
>>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>>>       .fops           = &at91wdt_fops,
>>>  };
>>>
>>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>>> +{
>>> +     if (!node)
>>> +             return;
>>> +
>>> +     of_property_read_u32(node, "timeout", &heartbeat);
>>> +}
>>> +
>>
>> In patch #1 you add a function to do this, and then you don't make use
>> of it here ?
>>
>> Or am i missing something?
>
> I'm using it on the patch #2 for the orion_wdt driver.
> Do you think it's better to join the #1 and the #2 patch?
>
> Best regards
> --
> Fabio Porcedda

I'm sorry, only now i understand your question.
The at91sam9_wdt driver don't use the watchdog core framework si i
can't use that function cleanly.
The patch #1 and #2 are for introducing the same property as the
at91sam9_wdt driver.
When the at91sam9_wdt is converted to the watchdog core framework, the
driver can use finally the new function.

Best regards
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>>> Tested on an at91sam9260 board (evk-pro3)
>>>
>>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>>> ---
>>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>>>  2 files changed, 40 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>> new file mode 100644
>>> index 0000000..65c1755
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>> @@ -0,0 +1,19 @@
>>> +* Atmel Watchdog Timers
>>> +
>>> +** at91sam9-wdt
>>> +
>>> +Required properties:
>>> +- compatible: must be "atmel,at91sam9260-wdt".
>>> +- reg: physical base address of the controller and length of memory mapped
>>> +  region.
>>> +
>>> +Optional properties:
>>> +- timeout: contains the watchdog timeout in seconds.
>>> +
>>> +Example:
>>> +
>>> +     watchdog@fffffd40 {
>>> +             compatible = "atmel,at91sam9260-wdt";
>>> +             reg = <0xfffffd40 0x10>;
>>> +             timeout = <10>;
>>> +     };
>>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>>> index 05e1be8..c9e6bfa 100644
>>> --- a/drivers/watchdog/at91sam9_wdt.c
>>> +++ b/drivers/watchdog/at91sam9_wdt.c
>>> @@ -32,6 +32,7 @@
>>>  #include <linux/timer.h>
>>>  #include <linux/bitops.h>
>>>  #include <linux/uaccess.h>
>>> +#include <linux/of.h>
>>>
>>>  #include "at91sam9_wdt.h"
>>>
>>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>>>       .fops           = &at91wdt_fops,
>>>  };
>>>
>>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>>> +{
>>> +     if (!node)
>>> +             return;
>>> +
>>> +     of_property_read_u32(node, "timeout", &heartbeat);
>>> +}
>>> +
>>
>> In patch #1 you add a function to do this, and then you don't make use
>> of it here ?
>>
>> Or am i missing something?
>
> I'm using it on the patch #2 for the orion_wdt driver.
> Do you think it's better to join the #1 and the #2 patch?
>
> Best regards
> --
> Fabio Porcedda

I'm sorry, only now i understand your question.
The at91sam9_wdt driver don't use the watchdog core framework si i
can't use that function cleanly.
The patch #1 and #2 are for introducing the same property as the
at91sam9_wdt driver.
When the at91sam9_wdt is converted to the watchdog core framework, the
driver can use finally the new function.

Best regards
-- 
Fabio Porcedda

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>>> Tested on an at91sam9260 board (evk-pro3)
>>>
>>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>>> ---
>>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>>>  2 files changed, 40 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>> new file mode 100644
>>> index 0000000..65c1755
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>>> @@ -0,0 +1,19 @@
>>> +* Atmel Watchdog Timers
>>> +
>>> +** at91sam9-wdt
>>> +
>>> +Required properties:
>>> +- compatible: must be "atmel,at91sam9260-wdt".
>>> +- reg: physical base address of the controller and length of memory mapped
>>> +  region.
>>> +
>>> +Optional properties:
>>> +- timeout: contains the watchdog timeout in seconds.
>>> +
>>> +Example:
>>> +
>>> +     watchdog at fffffd40 {
>>> +             compatible = "atmel,at91sam9260-wdt";
>>> +             reg = <0xfffffd40 0x10>;
>>> +             timeout = <10>;
>>> +     };
>>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>>> index 05e1be8..c9e6bfa 100644
>>> --- a/drivers/watchdog/at91sam9_wdt.c
>>> +++ b/drivers/watchdog/at91sam9_wdt.c
>>> @@ -32,6 +32,7 @@
>>>  #include <linux/timer.h>
>>>  #include <linux/bitops.h>
>>>  #include <linux/uaccess.h>
>>> +#include <linux/of.h>
>>>
>>>  #include "at91sam9_wdt.h"
>>>
>>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>>>       .fops           = &at91wdt_fops,
>>>  };
>>>
>>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>>> +{
>>> +     if (!node)
>>> +             return;
>>> +
>>> +     of_property_read_u32(node, "timeout", &heartbeat);
>>> +}
>>> +
>>
>> In patch #1 you add a function to do this, and then you don't make use
>> of it here ?
>>
>> Or am i missing something?
>
> I'm using it on the patch #2 for the orion_wdt driver.
> Do you think it's better to join the #1 and the #2 patch?
>
> Best regards
> --
> Fabio Porcedda

I'm sorry, only now i understand your question.
The at91sam9_wdt driver don't use the watchdog core framework si i
can't use that function cleanly.
The patch #1 and #2 are for introducing the same property as the
at91sam9_wdt driver.
When the at91sam9_wdt is converted to the watchdog core framework, the
driver can use finally the new function.

Best regards
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:48:01PM +0200, Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> > On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> Tested on an at91sam9260 board (evk-pro3)
> >>
> >> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> ---
> >>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >>  2 files changed, 40 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> new file mode 100644
> >> index 0000000..65c1755
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> @@ -0,0 +1,19 @@
> >> +* Atmel Watchdog Timers
> >> +
> >> +** at91sam9-wdt
> >> +
> >> +Required properties:
> >> +- compatible: must be "atmel,at91sam9260-wdt".
> >> +- reg: physical base address of the controller and length of memory mapped
> >> +  region.
> >> +
> >> +Optional properties:
> >> +- timeout: contains the watchdog timeout in seconds.
> >> +
> >> +Example:
> >> +
> >> +     watchdog@fffffd40 {
> >> +             compatible = "atmel,at91sam9260-wdt";
> >> +             reg = <0xfffffd40 0x10>;
> >> +             timeout = <10>;
> >> +     };
> >> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >> index 05e1be8..c9e6bfa 100644
> >> --- a/drivers/watchdog/at91sam9_wdt.c
> >> +++ b/drivers/watchdog/at91sam9_wdt.c
> >> @@ -32,6 +32,7 @@
> >>  #include <linux/timer.h>
> >>  #include <linux/bitops.h>
> >>  #include <linux/uaccess.h>
> >> +#include <linux/of.h>
> >>
> >>  #include "at91sam9_wdt.h"
> >>
> >> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >>       .fops           = &at91wdt_fops,
> >>  };
> >>
> >> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >> +{
> >> +     if (!node)
> >> +             return;
> >> +
> >> +     of_property_read_u32(node, "timeout", &heartbeat);
> >> +}
> >> +
> >
> > In patch #1 you add a function to do this, and then you don't make use
> > of it here ?
> >
> > Or am i missing something?
> 
> I'm using it on the patch #2 for the orion_wdt driver.
> Do you think it's better to join the #1 and the #2 patch?

It makes little sense doing this here, because you are going to have
to clean this up sometime very soon and use the helper, unless there
is a very good reason not to use the helper.

   Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:48:01PM +0200, Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> Tested on an at91sam9260 board (evk-pro3)
> >>
> >> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> ---
> >>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >>  2 files changed, 40 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> new file mode 100644
> >> index 0000000..65c1755
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> @@ -0,0 +1,19 @@
> >> +* Atmel Watchdog Timers
> >> +
> >> +** at91sam9-wdt
> >> +
> >> +Required properties:
> >> +- compatible: must be "atmel,at91sam9260-wdt".
> >> +- reg: physical base address of the controller and length of memory mapped
> >> +  region.
> >> +
> >> +Optional properties:
> >> +- timeout: contains the watchdog timeout in seconds.
> >> +
> >> +Example:
> >> +
> >> +     watchdog@fffffd40 {
> >> +             compatible = "atmel,at91sam9260-wdt";
> >> +             reg = <0xfffffd40 0x10>;
> >> +             timeout = <10>;
> >> +     };
> >> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >> index 05e1be8..c9e6bfa 100644
> >> --- a/drivers/watchdog/at91sam9_wdt.c
> >> +++ b/drivers/watchdog/at91sam9_wdt.c
> >> @@ -32,6 +32,7 @@
> >>  #include <linux/timer.h>
> >>  #include <linux/bitops.h>
> >>  #include <linux/uaccess.h>
> >> +#include <linux/of.h>
> >>
> >>  #include "at91sam9_wdt.h"
> >>
> >> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >>       .fops           = &at91wdt_fops,
> >>  };
> >>
> >> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >> +{
> >> +     if (!node)
> >> +             return;
> >> +
> >> +     of_property_read_u32(node, "timeout", &heartbeat);
> >> +}
> >> +
> >
> > In patch #1 you add a function to do this, and then you don't make use
> > of it here ?
> >
> > Or am i missing something?
> 
> I'm using it on the patch #2 for the orion_wdt driver.
> Do you think it's better to join the #1 and the #2 patch?

It makes little sense doing this here, because you are going to have
to clean this up sometime very soon and use the helper, unless there
is a very good reason not to use the helper.

   Andrew

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:48:01PM +0200, Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> Tested on an at91sam9260 board (evk-pro3)
> >>
> >> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> ---
> >>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >>  2 files changed, 40 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> new file mode 100644
> >> index 0000000..65c1755
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> @@ -0,0 +1,19 @@
> >> +* Atmel Watchdog Timers
> >> +
> >> +** at91sam9-wdt
> >> +
> >> +Required properties:
> >> +- compatible: must be "atmel,at91sam9260-wdt".
> >> +- reg: physical base address of the controller and length of memory mapped
> >> +  region.
> >> +
> >> +Optional properties:
> >> +- timeout: contains the watchdog timeout in seconds.
> >> +
> >> +Example:
> >> +
> >> +     watchdog at fffffd40 {
> >> +             compatible = "atmel,at91sam9260-wdt";
> >> +             reg = <0xfffffd40 0x10>;
> >> +             timeout = <10>;
> >> +     };
> >> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >> index 05e1be8..c9e6bfa 100644
> >> --- a/drivers/watchdog/at91sam9_wdt.c
> >> +++ b/drivers/watchdog/at91sam9_wdt.c
> >> @@ -32,6 +32,7 @@
> >>  #include <linux/timer.h>
> >>  #include <linux/bitops.h>
> >>  #include <linux/uaccess.h>
> >> +#include <linux/of.h>
> >>
> >>  #include "at91sam9_wdt.h"
> >>
> >> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >>       .fops           = &at91wdt_fops,
> >>  };
> >>
> >> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >> +{
> >> +     if (!node)
> >> +             return;
> >> +
> >> +     of_property_read_u32(node, "timeout", &heartbeat);
> >> +}
> >> +
> >
> > In patch #1 you add a function to do this, and then you don't make use
> > of it here ?
> >
> > Or am i missing something?
> 
> I'm using it on the patch #2 for the orion_wdt driver.
> Do you think it's better to join the #1 and the #2 patch?

It makes little sense doing this here, because you are going to have
to clean this up sometime very soon and use the helper, unless there
is a very good reason not to use the helper.

   Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >>> Tested on an at91sam9260 board (evk-pro3)
> >>>
> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>> ---
> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >>>  2 files changed, 40 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>> new file mode 100644
> >>> index 0000000..65c1755
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>> @@ -0,0 +1,19 @@
> >>> +* Atmel Watchdog Timers
> >>> +
> >>> +** at91sam9-wdt
> >>> +
> >>> +Required properties:
> >>> +- compatible: must be "atmel,at91sam9260-wdt".
> >>> +- reg: physical base address of the controller and length of memory mapped
> >>> +  region.
> >>> +
> >>> +Optional properties:
> >>> +- timeout: contains the watchdog timeout in seconds.
> >>> +
> >>> +Example:
> >>> +
> >>> +     watchdog@fffffd40 {
> >>> +             compatible = "atmel,at91sam9260-wdt";
> >>> +             reg = <0xfffffd40 0x10>;
> >>> +             timeout = <10>;
> >>> +     };
> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >>> index 05e1be8..c9e6bfa 100644
> >>> --- a/drivers/watchdog/at91sam9_wdt.c
> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
> >>> @@ -32,6 +32,7 @@
> >>>  #include <linux/timer.h>
> >>>  #include <linux/bitops.h>
> >>>  #include <linux/uaccess.h>
> >>> +#include <linux/of.h>
> >>>
> >>>  #include "at91sam9_wdt.h"
> >>>
> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >>>       .fops           = &at91wdt_fops,
> >>>  };
> >>>
> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >>> +{
> >>> +     if (!node)
> >>> +             return;
> >>> +
> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
> >>> +}
> >>> +
> >>
> >> In patch #1 you add a function to do this, and then you don't make use
> >> of it here ?
> >>
> >> Or am i missing something?
> >
> > I'm using it on the patch #2 for the orion_wdt driver.
> > Do you think it's better to join the #1 and the #2 patch?
> >
> > Best regards
> > --
> > Fabio Porcedda
> 
> I'm sorry, only now i understand your question.
> The at91sam9_wdt driver don't use the watchdog core framework si i
> can't use that function cleanly.

> The patch #1 and #2 are for introducing the same property as the
> at91sam9_wdt driver.

So maybe split this up into two different patchsets. One patchset to
add the helper function, and the use of this helper to all watchdog
divers that can use it. I think the following drivers should be
modified:

orion_wdt.c
pnx4008_wdt.c
s3c2410_wdt.c

In a second patchset, convert the AT91SAM9 driver over to the watchdog
core framework, and then use the helper function.

     Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >>> Tested on an at91sam9260 board (evk-pro3)
> >>>
> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >>> ---
> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >>>  2 files changed, 40 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>> new file mode 100644
> >>> index 0000000..65c1755
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>> @@ -0,0 +1,19 @@
> >>> +* Atmel Watchdog Timers
> >>> +
> >>> +** at91sam9-wdt
> >>> +
> >>> +Required properties:
> >>> +- compatible: must be "atmel,at91sam9260-wdt".
> >>> +- reg: physical base address of the controller and length of memory mapped
> >>> +  region.
> >>> +
> >>> +Optional properties:
> >>> +- timeout: contains the watchdog timeout in seconds.
> >>> +
> >>> +Example:
> >>> +
> >>> +     watchdog@fffffd40 {
> >>> +             compatible = "atmel,at91sam9260-wdt";
> >>> +             reg = <0xfffffd40 0x10>;
> >>> +             timeout = <10>;
> >>> +     };
> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >>> index 05e1be8..c9e6bfa 100644
> >>> --- a/drivers/watchdog/at91sam9_wdt.c
> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
> >>> @@ -32,6 +32,7 @@
> >>>  #include <linux/timer.h>
> >>>  #include <linux/bitops.h>
> >>>  #include <linux/uaccess.h>
> >>> +#include <linux/of.h>
> >>>
> >>>  #include "at91sam9_wdt.h"
> >>>
> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >>>       .fops           = &at91wdt_fops,
> >>>  };
> >>>
> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >>> +{
> >>> +     if (!node)
> >>> +             return;
> >>> +
> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
> >>> +}
> >>> +
> >>
> >> In patch #1 you add a function to do this, and then you don't make use
> >> of it here ?
> >>
> >> Or am i missing something?
> >
> > I'm using it on the patch #2 for the orion_wdt driver.
> > Do you think it's better to join the #1 and the #2 patch?
> >
> > Best regards
> > --
> > Fabio Porcedda
> 
> I'm sorry, only now i understand your question.
> The at91sam9_wdt driver don't use the watchdog core framework si i
> can't use that function cleanly.

> The patch #1 and #2 are for introducing the same property as the
> at91sam9_wdt driver.

So maybe split this up into two different patchsets. One patchset to
add the helper function, and the use of this helper to all watchdog
divers that can use it. I think the following drivers should be
modified:

orion_wdt.c
pnx4008_wdt.c
s3c2410_wdt.c

In a second patchset, convert the AT91SAM9 driver over to the watchdog
core framework, and then use the helper function.

     Andrew

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >>> Tested on an at91sam9260 board (evk-pro3)
> >>>
> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >>> ---
> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >>>  2 files changed, 40 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>> new file mode 100644
> >>> index 0000000..65c1755
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >>> @@ -0,0 +1,19 @@
> >>> +* Atmel Watchdog Timers
> >>> +
> >>> +** at91sam9-wdt
> >>> +
> >>> +Required properties:
> >>> +- compatible: must be "atmel,at91sam9260-wdt".
> >>> +- reg: physical base address of the controller and length of memory mapped
> >>> +  region.
> >>> +
> >>> +Optional properties:
> >>> +- timeout: contains the watchdog timeout in seconds.
> >>> +
> >>> +Example:
> >>> +
> >>> +     watchdog at fffffd40 {
> >>> +             compatible = "atmel,at91sam9260-wdt";
> >>> +             reg = <0xfffffd40 0x10>;
> >>> +             timeout = <10>;
> >>> +     };
> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >>> index 05e1be8..c9e6bfa 100644
> >>> --- a/drivers/watchdog/at91sam9_wdt.c
> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
> >>> @@ -32,6 +32,7 @@
> >>>  #include <linux/timer.h>
> >>>  #include <linux/bitops.h>
> >>>  #include <linux/uaccess.h>
> >>> +#include <linux/of.h>
> >>>
> >>>  #include "at91sam9_wdt.h"
> >>>
> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >>>       .fops           = &at91wdt_fops,
> >>>  };
> >>>
> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >>> +{
> >>> +     if (!node)
> >>> +             return;
> >>> +
> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
> >>> +}
> >>> +
> >>
> >> In patch #1 you add a function to do this, and then you don't make use
> >> of it here ?
> >>
> >> Or am i missing something?
> >
> > I'm using it on the patch #2 for the orion_wdt driver.
> > Do you think it's better to join the #1 and the #2 patch?
> >
> > Best regards
> > --
> > Fabio Porcedda
> 
> I'm sorry, only now i understand your question.
> The at91sam9_wdt driver don't use the watchdog core framework si i
> can't use that function cleanly.

> The patch #1 and #2 are for introducing the same property as the
> at91sam9_wdt driver.

So maybe split this up into two different patchsets. One patchset to
add the helper function, and the use of this helper to all watchdog
divers that can use it. I think the following drivers should be
modified:

orion_wdt.c
pnx4008_wdt.c
s3c2410_wdt.c

In a second patchset, convert the AT91SAM9 driver over to the watchdog
core framework, and then use the helper function.

     Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
>> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> >>> Tested on an at91sam9260 board (evk-pro3)
>> >>>
>> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >>> ---
>> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>> >>>  2 files changed, 40 insertions(+)
>> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>>
>> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>> new file mode 100644
>> >>> index 0000000..65c1755
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>> @@ -0,0 +1,19 @@
>> >>> +* Atmel Watchdog Timers
>> >>> +
>> >>> +** at91sam9-wdt
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: must be "atmel,at91sam9260-wdt".
>> >>> +- reg: physical base address of the controller and length of memory mapped
>> >>> +  region.
>> >>> +
>> >>> +Optional properties:
>> >>> +- timeout: contains the watchdog timeout in seconds.
>> >>> +
>> >>> +Example:
>> >>> +
>> >>> +     watchdog@fffffd40 {
>> >>> +             compatible = "atmel,at91sam9260-wdt";
>> >>> +             reg = <0xfffffd40 0x10>;
>> >>> +             timeout = <10>;
>> >>> +     };
>> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>> >>> index 05e1be8..c9e6bfa 100644
>> >>> --- a/drivers/watchdog/at91sam9_wdt.c
>> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
>> >>> @@ -32,6 +32,7 @@
>> >>>  #include <linux/timer.h>
>> >>>  #include <linux/bitops.h>
>> >>>  #include <linux/uaccess.h>
>> >>> +#include <linux/of.h>
>> >>>
>> >>>  #include "at91sam9_wdt.h"
>> >>>
>> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>> >>>       .fops           = &at91wdt_fops,
>> >>>  };
>> >>>
>> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>> >>> +{
>> >>> +     if (!node)
>> >>> +             return;
>> >>> +
>> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
>> >>> +}
>> >>> +
>> >>
>> >> In patch #1 you add a function to do this, and then you don't make use
>> >> of it here ?
>> >>
>> >> Or am i missing something?
>> >
>> > I'm using it on the patch #2 for the orion_wdt driver.
>> > Do you think it's better to join the #1 and the #2 patch?
>> >
>> > Best regards
>> > --
>> > Fabio Porcedda
>>
>> I'm sorry, only now i understand your question.
>> The at91sam9_wdt driver don't use the watchdog core framework si i
>> can't use that function cleanly.
>
>> The patch #1 and #2 are for introducing the same property as the
>> at91sam9_wdt driver.
>
> So maybe split this up into two different patchsets. One patchset to
> add the helper function, and the use of this helper to all watchdog
> divers that can use it. I think the following drivers should be
> modified:
>
> orion_wdt.c
> pnx4008_wdt.c
> s3c2410_wdt.c
>
> In a second patchset, convert the AT91SAM9 driver over to the watchdog
> core framework, and then use the helper function.

I was thinking to add a more generic helper function like this:

static inline void watchdog_get_dttimeout(struct device_node *node,
u32 *timeout)
{
	if (node)
		of_property_read_u32(node, "timeout", &wdd->timeout);
}

This way i can use this helper function in the at91sam9_wdt driver too.
What do you think?

Best regards
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
>> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
>> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> >>> Tested on an at91sam9260 board (evk-pro3)
>> >>>
>> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> >>> ---
>> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>> >>>  2 files changed, 40 insertions(+)
>> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>>
>> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>> new file mode 100644
>> >>> index 0000000..65c1755
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>> @@ -0,0 +1,19 @@
>> >>> +* Atmel Watchdog Timers
>> >>> +
>> >>> +** at91sam9-wdt
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: must be "atmel,at91sam9260-wdt".
>> >>> +- reg: physical base address of the controller and length of memory mapped
>> >>> +  region.
>> >>> +
>> >>> +Optional properties:
>> >>> +- timeout: contains the watchdog timeout in seconds.
>> >>> +
>> >>> +Example:
>> >>> +
>> >>> +     watchdog@fffffd40 {
>> >>> +             compatible = "atmel,at91sam9260-wdt";
>> >>> +             reg = <0xfffffd40 0x10>;
>> >>> +             timeout = <10>;
>> >>> +     };
>> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>> >>> index 05e1be8..c9e6bfa 100644
>> >>> --- a/drivers/watchdog/at91sam9_wdt.c
>> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
>> >>> @@ -32,6 +32,7 @@
>> >>>  #include <linux/timer.h>
>> >>>  #include <linux/bitops.h>
>> >>>  #include <linux/uaccess.h>
>> >>> +#include <linux/of.h>
>> >>>
>> >>>  #include "at91sam9_wdt.h"
>> >>>
>> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>> >>>       .fops           = &at91wdt_fops,
>> >>>  };
>> >>>
>> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>> >>> +{
>> >>> +     if (!node)
>> >>> +             return;
>> >>> +
>> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
>> >>> +}
>> >>> +
>> >>
>> >> In patch #1 you add a function to do this, and then you don't make use
>> >> of it here ?
>> >>
>> >> Or am i missing something?
>> >
>> > I'm using it on the patch #2 for the orion_wdt driver.
>> > Do you think it's better to join the #1 and the #2 patch?
>> >
>> > Best regards
>> > --
>> > Fabio Porcedda
>>
>> I'm sorry, only now i understand your question.
>> The at91sam9_wdt driver don't use the watchdog core framework si i
>> can't use that function cleanly.
>
>> The patch #1 and #2 are for introducing the same property as the
>> at91sam9_wdt driver.
>
> So maybe split this up into two different patchsets. One patchset to
> add the helper function, and the use of this helper to all watchdog
> divers that can use it. I think the following drivers should be
> modified:
>
> orion_wdt.c
> pnx4008_wdt.c
> s3c2410_wdt.c
>
> In a second patchset, convert the AT91SAM9 driver over to the watchdog
> core framework, and then use the helper function.

I was thinking to add a more generic helper function like this:

static inline void watchdog_get_dttimeout(struct device_node *node,
u32 *timeout)
{
	if (node)
		of_property_read_u32(node, "timeout", &wdd->timeout);
}

This way i can use this helper function in the at91sam9_wdt driver too.
What do you think?

Best regards
-- 
Fabio Porcedda

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
>> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
>> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> >>> Tested on an at91sam9260 board (evk-pro3)
>> >>>
>> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> >>> ---
>> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>> >>>  2 files changed, 40 insertions(+)
>> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>>
>> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>> new file mode 100644
>> >>> index 0000000..65c1755
>> >>> --- /dev/null
>> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
>> >>> @@ -0,0 +1,19 @@
>> >>> +* Atmel Watchdog Timers
>> >>> +
>> >>> +** at91sam9-wdt
>> >>> +
>> >>> +Required properties:
>> >>> +- compatible: must be "atmel,at91sam9260-wdt".
>> >>> +- reg: physical base address of the controller and length of memory mapped
>> >>> +  region.
>> >>> +
>> >>> +Optional properties:
>> >>> +- timeout: contains the watchdog timeout in seconds.
>> >>> +
>> >>> +Example:
>> >>> +
>> >>> +     watchdog at fffffd40 {
>> >>> +             compatible = "atmel,at91sam9260-wdt";
>> >>> +             reg = <0xfffffd40 0x10>;
>> >>> +             timeout = <10>;
>> >>> +     };
>> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
>> >>> index 05e1be8..c9e6bfa 100644
>> >>> --- a/drivers/watchdog/at91sam9_wdt.c
>> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
>> >>> @@ -32,6 +32,7 @@
>> >>>  #include <linux/timer.h>
>> >>>  #include <linux/bitops.h>
>> >>>  #include <linux/uaccess.h>
>> >>> +#include <linux/of.h>
>> >>>
>> >>>  #include "at91sam9_wdt.h"
>> >>>
>> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
>> >>>       .fops           = &at91wdt_fops,
>> >>>  };
>> >>>
>> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
>> >>> +{
>> >>> +     if (!node)
>> >>> +             return;
>> >>> +
>> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
>> >>> +}
>> >>> +
>> >>
>> >> In patch #1 you add a function to do this, and then you don't make use
>> >> of it here ?
>> >>
>> >> Or am i missing something?
>> >
>> > I'm using it on the patch #2 for the orion_wdt driver.
>> > Do you think it's better to join the #1 and the #2 patch?
>> >
>> > Best regards
>> > --
>> > Fabio Porcedda
>>
>> I'm sorry, only now i understand your question.
>> The at91sam9_wdt driver don't use the watchdog core framework si i
>> can't use that function cleanly.
>
>> The patch #1 and #2 are for introducing the same property as the
>> at91sam9_wdt driver.
>
> So maybe split this up into two different patchsets. One patchset to
> add the helper function, and the use of this helper to all watchdog
> divers that can use it. I think the following drivers should be
> modified:
>
> orion_wdt.c
> pnx4008_wdt.c
> s3c2410_wdt.c
>
> In a second patchset, convert the AT91SAM9 driver over to the watchdog
> core framework, and then use the helper function.

I was thinking to add a more generic helper function like this:

static inline void watchdog_get_dttimeout(struct device_node *node,
u32 *timeout)
{
	if (node)
		of_property_read_u32(node, "timeout", &wdd->timeout);
}

This way i can use this helper function in the at91sam9_wdt driver too.
What do you think?

Best regards
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> >>> Tested on an at91sam9260 board (evk-pro3)
> >> >>>
> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> >>> ---
> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >> >>>  2 files changed, 40 insertions(+)
> >> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>>
> >> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>> new file mode 100644
> >> >>> index 0000000..65c1755
> >> >>> --- /dev/null
> >> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>> @@ -0,0 +1,19 @@
> >> >>> +* Atmel Watchdog Timers
> >> >>> +
> >> >>> +** at91sam9-wdt
> >> >>> +
> >> >>> +Required properties:
> >> >>> +- compatible: must be "atmel,at91sam9260-wdt".
> >> >>> +- reg: physical base address of the controller and length of memory mapped
> >> >>> +  region.
> >> >>> +
> >> >>> +Optional properties:
> >> >>> +- timeout: contains the watchdog timeout in seconds.
> >> >>> +
> >> >>> +Example:
> >> >>> +
> >> >>> +     watchdog@fffffd40 {
> >> >>> +             compatible = "atmel,at91sam9260-wdt";
> >> >>> +             reg = <0xfffffd40 0x10>;
> >> >>> +             timeout = <10>;
> >> >>> +     };
> >> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >> >>> index 05e1be8..c9e6bfa 100644
> >> >>> --- a/drivers/watchdog/at91sam9_wdt.c
> >> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
> >> >>> @@ -32,6 +32,7 @@
> >> >>>  #include <linux/timer.h>
> >> >>>  #include <linux/bitops.h>
> >> >>>  #include <linux/uaccess.h>
> >> >>> +#include <linux/of.h>
> >> >>>
> >> >>>  #include "at91sam9_wdt.h"
> >> >>>
> >> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >> >>>       .fops           = &at91wdt_fops,
> >> >>>  };
> >> >>>
> >> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >> >>> +{
> >> >>> +     if (!node)
> >> >>> +             return;
> >> >>> +
> >> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
> >> >>> +}
> >> >>> +
> >> >>
> >> >> In patch #1 you add a function to do this, and then you don't make use
> >> >> of it here ?
> >> >>
> >> >> Or am i missing something?
> >> >
> >> > I'm using it on the patch #2 for the orion_wdt driver.
> >> > Do you think it's better to join the #1 and the #2 patch?
> >> >
> >> > Best regards
> >> > --
> >> > Fabio Porcedda
> >>
> >> I'm sorry, only now i understand your question.
> >> The at91sam9_wdt driver don't use the watchdog core framework si i
> >> can't use that function cleanly.
> >
> >> The patch #1 and #2 are for introducing the same property as the
> >> at91sam9_wdt driver.
> >
> > So maybe split this up into two different patchsets. One patchset to
> > add the helper function, and the use of this helper to all watchdog
> > divers that can use it. I think the following drivers should be
> > modified:
> >
> > orion_wdt.c
> > pnx4008_wdt.c
> > s3c2410_wdt.c
> >
> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
> > core framework, and then use the helper function.
> 
> I was thinking to add a more generic helper function like this:
> 
> static inline void watchdog_get_dttimeout(struct device_node *node,
> u32 *timeout)
> {
> 	if (node)
> 		of_property_read_u32(node, "timeout", &wdd->timeout);
> }
> 
> This way i can use this helper function in the at91sam9_wdt driver too.
> What do you think?
timeout_sec and this can be move at of.h level

as this is not watchdog framework secific

Best Regards,
J.

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> >>> Tested on an at91sam9260 board (evk-pro3)
> >> >>>
> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> >>> ---
> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >> >>>  2 files changed, 40 insertions(+)
> >> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>>
> >> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>> new file mode 100644
> >> >>> index 0000000..65c1755
> >> >>> --- /dev/null
> >> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>> @@ -0,0 +1,19 @@
> >> >>> +* Atmel Watchdog Timers
> >> >>> +
> >> >>> +** at91sam9-wdt
> >> >>> +
> >> >>> +Required properties:
> >> >>> +- compatible: must be "atmel,at91sam9260-wdt".
> >> >>> +- reg: physical base address of the controller and length of memory mapped
> >> >>> +  region.
> >> >>> +
> >> >>> +Optional properties:
> >> >>> +- timeout: contains the watchdog timeout in seconds.
> >> >>> +
> >> >>> +Example:
> >> >>> +
> >> >>> +     watchdog@fffffd40 {
> >> >>> +             compatible = "atmel,at91sam9260-wdt";
> >> >>> +             reg = <0xfffffd40 0x10>;
> >> >>> +             timeout = <10>;
> >> >>> +     };
> >> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >> >>> index 05e1be8..c9e6bfa 100644
> >> >>> --- a/drivers/watchdog/at91sam9_wdt.c
> >> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
> >> >>> @@ -32,6 +32,7 @@
> >> >>>  #include <linux/timer.h>
> >> >>>  #include <linux/bitops.h>
> >> >>>  #include <linux/uaccess.h>
> >> >>> +#include <linux/of.h>
> >> >>>
> >> >>>  #include "at91sam9_wdt.h"
> >> >>>
> >> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >> >>>       .fops           = &at91wdt_fops,
> >> >>>  };
> >> >>>
> >> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >> >>> +{
> >> >>> +     if (!node)
> >> >>> +             return;
> >> >>> +
> >> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
> >> >>> +}
> >> >>> +
> >> >>
> >> >> In patch #1 you add a function to do this, and then you don't make use
> >> >> of it here ?
> >> >>
> >> >> Or am i missing something?
> >> >
> >> > I'm using it on the patch #2 for the orion_wdt driver.
> >> > Do you think it's better to join the #1 and the #2 patch?
> >> >
> >> > Best regards
> >> > --
> >> > Fabio Porcedda
> >>
> >> I'm sorry, only now i understand your question.
> >> The at91sam9_wdt driver don't use the watchdog core framework si i
> >> can't use that function cleanly.
> >
> >> The patch #1 and #2 are for introducing the same property as the
> >> at91sam9_wdt driver.
> >
> > So maybe split this up into two different patchsets. One patchset to
> > add the helper function, and the use of this helper to all watchdog
> > divers that can use it. I think the following drivers should be
> > modified:
> >
> > orion_wdt.c
> > pnx4008_wdt.c
> > s3c2410_wdt.c
> >
> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
> > core framework, and then use the helper function.
> 
> I was thinking to add a more generic helper function like this:
> 
> static inline void watchdog_get_dttimeout(struct device_node *node,
> u32 *timeout)
> {
> 	if (node)
> 		of_property_read_u32(node, "timeout", &wdd->timeout);
> }
> 
> This way i can use this helper function in the at91sam9_wdt driver too.
> What do you think?
timeout_sec and this can be move at of.h level

as this is not watchdog framework secific

Best Regards,
J.

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> >>> Tested on an at91sam9260 board (evk-pro3)
> >> >>>
> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> >>> ---
> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >> >>>  2 files changed, 40 insertions(+)
> >> >>>  create mode 100644 Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>>
> >> >>> diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>> new file mode 100644
> >> >>> index 0000000..65c1755
> >> >>> --- /dev/null
> >> >>> +++ b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
> >> >>> @@ -0,0 +1,19 @@
> >> >>> +* Atmel Watchdog Timers
> >> >>> +
> >> >>> +** at91sam9-wdt
> >> >>> +
> >> >>> +Required properties:
> >> >>> +- compatible: must be "atmel,at91sam9260-wdt".
> >> >>> +- reg: physical base address of the controller and length of memory mapped
> >> >>> +  region.
> >> >>> +
> >> >>> +Optional properties:
> >> >>> +- timeout: contains the watchdog timeout in seconds.
> >> >>> +
> >> >>> +Example:
> >> >>> +
> >> >>> +     watchdog at fffffd40 {
> >> >>> +             compatible = "atmel,at91sam9260-wdt";
> >> >>> +             reg = <0xfffffd40 0x10>;
> >> >>> +             timeout = <10>;
> >> >>> +     };
> >> >>> diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> >> >>> index 05e1be8..c9e6bfa 100644
> >> >>> --- a/drivers/watchdog/at91sam9_wdt.c
> >> >>> +++ b/drivers/watchdog/at91sam9_wdt.c
> >> >>> @@ -32,6 +32,7 @@
> >> >>>  #include <linux/timer.h>
> >> >>>  #include <linux/bitops.h>
> >> >>>  #include <linux/uaccess.h>
> >> >>> +#include <linux/of.h>
> >> >>>
> >> >>>  #include "at91sam9_wdt.h"
> >> >>>
> >> >>> @@ -254,6 +255,14 @@ static struct miscdevice at91wdt_miscdev = {
> >> >>>       .fops           = &at91wdt_fops,
> >> >>>  };
> >> >>>
> >> >>> +static inline void __init at91wdt_probe_dt(struct device_node *node)
> >> >>> +{
> >> >>> +     if (!node)
> >> >>> +             return;
> >> >>> +
> >> >>> +     of_property_read_u32(node, "timeout", &heartbeat);
> >> >>> +}
> >> >>> +
> >> >>
> >> >> In patch #1 you add a function to do this, and then you don't make use
> >> >> of it here ?
> >> >>
> >> >> Or am i missing something?
> >> >
> >> > I'm using it on the patch #2 for the orion_wdt driver.
> >> > Do you think it's better to join the #1 and the #2 patch?
> >> >
> >> > Best regards
> >> > --
> >> > Fabio Porcedda
> >>
> >> I'm sorry, only now i understand your question.
> >> The at91sam9_wdt driver don't use the watchdog core framework si i
> >> can't use that function cleanly.
> >
> >> The patch #1 and #2 are for introducing the same property as the
> >> at91sam9_wdt driver.
> >
> > So maybe split this up into two different patchsets. One patchset to
> > add the helper function, and the use of this helper to all watchdog
> > divers that can use it. I think the following drivers should be
> > modified:
> >
> > orion_wdt.c
> > pnx4008_wdt.c
> > s3c2410_wdt.c
> >
> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
> > core framework, and then use the helper function.
> 
> I was thinking to add a more generic helper function like this:
> 
> static inline void watchdog_get_dttimeout(struct device_node *node,
> u32 *timeout)
> {
> 	if (node)
> 		of_property_read_u32(node, "timeout", &wdd->timeout);
> }
> 
> This way i can use this helper function in the at91sam9_wdt driver too.
> What do you think?
timeout_sec and this can be move at of.h level

as this is not watchdog framework secific

Best Regards,
J.

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

> I was thinking to add a more generic helper function like this:
> 
> static inline void watchdog_get_dttimeout(struct device_node *node,
> u32 *timeout)
> {
> 	if (node)
> 		of_property_read_u32(node, "timeout", &wdd->timeout);
> }

You forgot to change the function signature. 

Also, if you are adding a generic function, it should be a generic
function for the framework. All drivers should be slowly moving
towards the framework, so adding functions which help you not move
towards the framework are wrong.

	Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

> I was thinking to add a more generic helper function like this:
> 
> static inline void watchdog_get_dttimeout(struct device_node *node,
> u32 *timeout)
> {
> 	if (node)
> 		of_property_read_u32(node, "timeout", &wdd->timeout);
> }

You forgot to change the function signature. 

Also, if you are adding a generic function, it should be a generic
function for the framework. All drivers should be slowly moving
towards the framework, so adding functions which help you not move
towards the framework are wrong.

	Andrew

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Andrew Lunn]

> I was thinking to add a more generic helper function like this:
> 
> static inline void watchdog_get_dttimeout(struct device_node *node,
> u32 *timeout)
> {
> 	if (node)
> 		of_property_read_u32(node, "timeout", &wdd->timeout);
> }

You forgot to change the function signature. 

Also, if you are adding a generic function, it should be a generic
function for the framework. All drivers should be slowly moving
towards the framework, so adding functions which help you not move
towards the framework are wrong.

	Andrew

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org> wrote:
> On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
>> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
>> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> >> >>> Tested on an at91sam9260 board (evk-pro3)
>> >> >>>
>> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >> >>> ---
>> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>> >> >>>  2 files changed, 40 insertions(+)
>> >> >>> ...
>> >> >>
>> >> >> In patch #1 you add a function to do this, and then you don't make use
>> >> >> of it here ?
>> >> >>
>> >> >> Or am i missing something?
>> >> >
>> >> > I'm using it on the patch #2 for the orion_wdt driver.
>> >> > Do you think it's better to join the #1 and the #2 patch?
>> >> >
>> >> > Best regards
>> >> > --
>> >> > Fabio Porcedda
>> >>
>> >> I'm sorry, only now i understand your question.
>> >> The at91sam9_wdt driver don't use the watchdog core framework si i
>> >> can't use that function cleanly.
>> >
>> >> The patch #1 and #2 are for introducing the same property as the
>> >> at91sam9_wdt driver.
>> >
>> > So maybe split this up into two different patchsets. One patchset to
>> > add the helper function, and the use of this helper to all watchdog
>> > divers that can use it. I think the following drivers should be
>> > modified:
>> >
>> > orion_wdt.c
>> > pnx4008_wdt.c
>> > s3c2410_wdt.c
>> >
>> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
>> > core framework, and then use the helper function.
>>
>> I was thinking to add a more generic helper function like this:
>>
>> static inline void watchdog_get_dttimeout(struct device_node *node,
>> u32 *timeout)
>> {
>>       if (node)
>>               of_property_read_u32(node, "timeout", &wdd->timeout);
>> }
>>
>> This way i can use this helper function in the at91sam9_wdt driver too.
>> What do you think?
> timeout_sec and this can be move at of.h level
>
> as this is not watchdog framework secific

I can not find any property with the "_sec" suffix, you think it's
still fine to use that suffix?

You are speaking about a of_watchdog.h header with a
of_watchdog_get_timeout function?

Best regards and thanks for the review.
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
>> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
>> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
>> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> >> >>> Tested on an at91sam9260 board (evk-pro3)
>> >> >>>
>> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> >> >>> ---
>> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>> >> >>>  2 files changed, 40 insertions(+)
>> >> >>> ...
>> >> >>
>> >> >> In patch #1 you add a function to do this, and then you don't make use
>> >> >> of it here ?
>> >> >>
>> >> >> Or am i missing something?
>> >> >
>> >> > I'm using it on the patch #2 for the orion_wdt driver.
>> >> > Do you think it's better to join the #1 and the #2 patch?
>> >> >
>> >> > Best regards
>> >> > --
>> >> > Fabio Porcedda
>> >>
>> >> I'm sorry, only now i understand your question.
>> >> The at91sam9_wdt driver don't use the watchdog core framework si i
>> >> can't use that function cleanly.
>> >
>> >> The patch #1 and #2 are for introducing the same property as the
>> >> at91sam9_wdt driver.
>> >
>> > So maybe split this up into two different patchsets. One patchset to
>> > add the helper function, and the use of this helper to all watchdog
>> > divers that can use it. I think the following drivers should be
>> > modified:
>> >
>> > orion_wdt.c
>> > pnx4008_wdt.c
>> > s3c2410_wdt.c
>> >
>> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
>> > core framework, and then use the helper function.
>>
>> I was thinking to add a more generic helper function like this:
>>
>> static inline void watchdog_get_dttimeout(struct device_node *node,
>> u32 *timeout)
>> {
>>       if (node)
>>               of_property_read_u32(node, "timeout", &wdd->timeout);
>> }
>>
>> This way i can use this helper function in the at91sam9_wdt driver too.
>> What do you think?
> timeout_sec and this can be move at of.h level
>
> as this is not watchdog framework secific

I can not find any property with the "_sec" suffix, you think it's
still fine to use that suffix?

You are speaking about a of_watchdog.h header with a
of_watchdog_get_timeout function?

Best regards and thanks for the review.
-- 
Fabio Porcedda

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
>> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
>> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
>> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
>> >> >>> Tested on an at91sam9260 board (evk-pro3)
>> >> >>>
>> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> >> >>> ---
>> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
>> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
>> >> >>>  2 files changed, 40 insertions(+)
>> >> >>> ...
>> >> >>
>> >> >> In patch #1 you add a function to do this, and then you don't make use
>> >> >> of it here ?
>> >> >>
>> >> >> Or am i missing something?
>> >> >
>> >> > I'm using it on the patch #2 for the orion_wdt driver.
>> >> > Do you think it's better to join the #1 and the #2 patch?
>> >> >
>> >> > Best regards
>> >> > --
>> >> > Fabio Porcedda
>> >>
>> >> I'm sorry, only now i understand your question.
>> >> The at91sam9_wdt driver don't use the watchdog core framework si i
>> >> can't use that function cleanly.
>> >
>> >> The patch #1 and #2 are for introducing the same property as the
>> >> at91sam9_wdt driver.
>> >
>> > So maybe split this up into two different patchsets. One patchset to
>> > add the helper function, and the use of this helper to all watchdog
>> > divers that can use it. I think the following drivers should be
>> > modified:
>> >
>> > orion_wdt.c
>> > pnx4008_wdt.c
>> > s3c2410_wdt.c
>> >
>> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
>> > core framework, and then use the helper function.
>>
>> I was thinking to add a more generic helper function like this:
>>
>> static inline void watchdog_get_dttimeout(struct device_node *node,
>> u32 *timeout)
>> {
>>       if (node)
>>               of_property_read_u32(node, "timeout", &wdd->timeout);
>> }
>>
>> This way i can use this helper function in the at91sam9_wdt driver too.
>> What do you think?
> timeout_sec and this can be move at of.h level
>
> as this is not watchdog framework secific

I can not find any property with the "_sec" suffix, you think it's
still fine to use that suffix?

You are speaking about a of_watchdog.h header with a
of_watchdog_get_timeout function?

Best regards and thanks for the review.
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Tue, Oct 2, 2012 at 9:59 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> I was thinking to add a more generic helper function like this:
>>
>> static inline void watchdog_get_dttimeout(struct device_node *node,
>> u32 *timeout)
>> {
>>       if (node)
>>               of_property_read_u32(node, "timeout", &wdd->timeout);
>> }
>
> You forgot to change the function signature.
>
> Also, if you are adding a generic function, it should be a generic
> function for the framework. All drivers should be slowly moving
> towards the framework, so adding functions which help you not move
> towards the framework are wrong.

Maybe i can use a framework specific function and use a dummy
watchdog_device for
the non framerwork drivers.

If it's ok i will send an updated patch.

Best regards and thanks for the review.
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Tue, Oct 2, 2012 at 9:59 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> I was thinking to add a more generic helper function like this:
>>
>> static inline void watchdog_get_dttimeout(struct device_node *node,
>> u32 *timeout)
>> {
>>       if (node)
>>               of_property_read_u32(node, "timeout", &wdd->timeout);
>> }
>
> You forgot to change the function signature.
>
> Also, if you are adding a generic function, it should be a generic
> function for the framework. All drivers should be slowly moving
> towards the framework, so adding functions which help you not move
> towards the framework are wrong.

Maybe i can use a framework specific function and use a dummy
watchdog_device for
the non framerwork drivers.

If it's ok i will send an updated patch.

Best regards and thanks for the review.
-- 
Fabio Porcedda

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Fabio Porcedda]

On Tue, Oct 2, 2012 at 9:59 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> I was thinking to add a more generic helper function like this:
>>
>> static inline void watchdog_get_dttimeout(struct device_node *node,
>> u32 *timeout)
>> {
>>       if (node)
>>               of_property_read_u32(node, "timeout", &wdd->timeout);
>> }
>
> You forgot to change the function signature.
>
> Also, if you are adding a generic function, it should be a generic
> function for the framework. All drivers should be slowly moving
> towards the framework, so adding functions which help you not move
> towards the framework are wrong.

Maybe i can use a framework specific function and use a dummy
watchdog_device for
the non framerwork drivers.

If it's ok i will send an updated patch.

Best regards and thanks for the review.
-- 
Fabio Porcedda

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:32 Wed 03 Oct     , Fabio Porcedda wrote:
> On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
> >> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> >> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> >> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> >> >>> Tested on an at91sam9260 board (evk-pro3)
> >> >> >>>
> >> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> >> >>> ---
> >> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >> >> >>>  2 files changed, 40 insertions(+)
> >> >> >>> ...
> >> >> >>
> >> >> >> In patch #1 you add a function to do this, and then you don't make use
> >> >> >> of it here ?
> >> >> >>
> >> >> >> Or am i missing something?
> >> >> >
> >> >> > I'm using it on the patch #2 for the orion_wdt driver.
> >> >> > Do you think it's better to join the #1 and the #2 patch?
> >> >> >
> >> >> > Best regards
> >> >> > --
> >> >> > Fabio Porcedda
> >> >>
> >> >> I'm sorry, only now i understand your question.
> >> >> The at91sam9_wdt driver don't use the watchdog core framework si i
> >> >> can't use that function cleanly.
> >> >
> >> >> The patch #1 and #2 are for introducing the same property as the
> >> >> at91sam9_wdt driver.
> >> >
> >> > So maybe split this up into two different patchsets. One patchset to
> >> > add the helper function, and the use of this helper to all watchdog
> >> > divers that can use it. I think the following drivers should be
> >> > modified:
> >> >
> >> > orion_wdt.c
> >> > pnx4008_wdt.c
> >> > s3c2410_wdt.c
> >> >
> >> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
> >> > core framework, and then use the helper function.
> >>
> >> I was thinking to add a more generic helper function like this:
> >>
> >> static inline void watchdog_get_dttimeout(struct device_node *node,
> >> u32 *timeout)
> >> {
> >>       if (node)
> >>               of_property_read_u32(node, "timeout", &wdd->timeout);
> >> }
> >>
> >> This way i can use this helper function in the at91sam9_wdt driver too.
> >> What do you think?
> > timeout_sec and this can be move at of.h level
> >
> > as this is not watchdog framework secific
> 
> I can not find any property with the "_sec" suffix, you think it's
> still fine to use that suffix?
> 
> You are speaking about a of_watchdog.h header with a
> of_watchdog_get_timeout function?
no a global helper not watchdog specific for timeout binding

that why I said of.h not of_watchdog.h

Best Regards,
J.

Re: [PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:32 Wed 03 Oct     , Fabio Porcedda wrote:
> On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
> >> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> >> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> >> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> >> >>> Tested on an at91sam9260 board (evk-pro3)
> >> >> >>>
> >> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> >> >>> ---
> >> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >> >> >>>  2 files changed, 40 insertions(+)
> >> >> >>> ...
> >> >> >>
> >> >> >> In patch #1 you add a function to do this, and then you don't make use
> >> >> >> of it here ?
> >> >> >>
> >> >> >> Or am i missing something?
> >> >> >
> >> >> > I'm using it on the patch #2 for the orion_wdt driver.
> >> >> > Do you think it's better to join the #1 and the #2 patch?
> >> >> >
> >> >> > Best regards
> >> >> > --
> >> >> > Fabio Porcedda
> >> >>
> >> >> I'm sorry, only now i understand your question.
> >> >> The at91sam9_wdt driver don't use the watchdog core framework si i
> >> >> can't use that function cleanly.
> >> >
> >> >> The patch #1 and #2 are for introducing the same property as the
> >> >> at91sam9_wdt driver.
> >> >
> >> > So maybe split this up into two different patchsets. One patchset to
> >> > add the helper function, and the use of this helper to all watchdog
> >> > divers that can use it. I think the following drivers should be
> >> > modified:
> >> >
> >> > orion_wdt.c
> >> > pnx4008_wdt.c
> >> > s3c2410_wdt.c
> >> >
> >> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
> >> > core framework, and then use the helper function.
> >>
> >> I was thinking to add a more generic helper function like this:
> >>
> >> static inline void watchdog_get_dttimeout(struct device_node *node,
> >> u32 *timeout)
> >> {
> >>       if (node)
> >>               of_property_read_u32(node, "timeout", &wdd->timeout);
> >> }
> >>
> >> This way i can use this helper function in the at91sam9_wdt driver too.
> >> What do you think?
> > timeout_sec and this can be move at of.h level
> >
> > as this is not watchdog framework secific
> 
> I can not find any property with the "_sec" suffix, you think it's
> still fine to use that suffix?
> 
> You are speaking about a of_watchdog.h header with a
> of_watchdog_get_timeout function?
no a global helper not watchdog specific for timeout binding

that why I said of.h not of_watchdog.h

Best Regards,
J.

[PATCH v6 3/5] watchdog: at91sam9_wdt: add device tree support

[Jean-Christophe PLAGNIOL-VILLARD]

On 18:32 Wed 03 Oct     , Fabio Porcedda wrote:
> On Tue, Oct 2, 2012 at 9:00 PM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 17:04 Tue 02 Oct     , Fabio Porcedda wrote:
> >> On Mon, Oct 1, 2012 at 3:06 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> > On Mon, Oct 01, 2012 at 02:54:55PM +0200, Fabio Porcedda wrote:
> >> >> On Mon, Oct 1, 2012 at 2:48 PM, Fabio Porcedda <fabio.porcedda@gmail.com> wrote:
> >> >> > On Mon, Oct 1, 2012 at 2:45 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> >> >> On Mon, Oct 01, 2012 at 02:24:39PM +0200, Fabio Porcedda wrote:
> >> >> >>> Tested on an at91sam9260 board (evk-pro3)
> >> >> >>>
> >> >> >>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> >> >> >>> ---
> >> >> >>>  .../devicetree/bindings/watchdog/atmel-wdt.txt      | 19 +++++++++++++++++++
> >> >> >>>  drivers/watchdog/at91sam9_wdt.c                     | 21 +++++++++++++++++++++
> >> >> >>>  2 files changed, 40 insertions(+)
> >> >> >>> ...
> >> >> >>
> >> >> >> In patch #1 you add a function to do this, and then you don't make use
> >> >> >> of it here ?
> >> >> >>
> >> >> >> Or am i missing something?
> >> >> >
> >> >> > I'm using it on the patch #2 for the orion_wdt driver.
> >> >> > Do you think it's better to join the #1 and the #2 patch?
> >> >> >
> >> >> > Best regards
> >> >> > --
> >> >> > Fabio Porcedda
> >> >>
> >> >> I'm sorry, only now i understand your question.
> >> >> The at91sam9_wdt driver don't use the watchdog core framework si i
> >> >> can't use that function cleanly.
> >> >
> >> >> The patch #1 and #2 are for introducing the same property as the
> >> >> at91sam9_wdt driver.
> >> >
> >> > So maybe split this up into two different patchsets. One patchset to
> >> > add the helper function, and the use of this helper to all watchdog
> >> > divers that can use it. I think the following drivers should be
> >> > modified:
> >> >
> >> > orion_wdt.c
> >> > pnx4008_wdt.c
> >> > s3c2410_wdt.c
> >> >
> >> > In a second patchset, convert the AT91SAM9 driver over to the watchdog
> >> > core framework, and then use the helper function.
> >>
> >> I was thinking to add a more generic helper function like this:
> >>
> >> static inline void watchdog_get_dttimeout(struct device_node *node,
> >> u32 *timeout)
> >> {
> >>       if (node)
> >>               of_property_read_u32(node, "timeout", &wdd->timeout);
> >> }
> >>
> >> This way i can use this helper function in the at91sam9_wdt driver too.
> >> What do you think?
> > timeout_sec and this can be move at of.h level
> >
> > as this is not watchdog framework secific
> 
> I can not find any property with the "_sec" suffix, you think it's
> still fine to use that suffix?
> 
> You are speaking about a of_watchdog.h header with a
> of_watchdog_get_timeout function?
no a global helper not watchdog specific for timeout binding

that why I said of.h not of_watchdog.h

Best Regards,
J.