diff mbox

[v4,1/2] watchdog: at91sam9_wdt: add device tree support

Message ID 1348568704-28304-2-git-send-email-fabio.porcedda@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fabio Porcedda Sept. 25, 2012, 10:25 a.m. UTC
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

Comments

Jean-Christophe PLAGNIOL-VILLARD Sept. 27, 2012, 5:56 a.m. UTC | #1
On 12:25 Tue 25 Sep     , 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..5947929
> --- /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:
> +- heartbeat: contains the watchdog heartbeats in seconds.
more I read this binding more it's seems wrong

Today we do not use yet the watchdog framework which we will so we need to
use the same binding as the watchdog framework

So when we switch the DT will not change

Best Regards,
J.
Fabio Porcedda Sept. 27, 2012, 2:59 p.m. UTC | #2
On Thu, Sep 27, 2012 at 7:56 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 12:25 Tue 25 Sep     , 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..5947929
>> --- /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:
>> +- heartbeat: contains the watchdog heartbeats in seconds.
> more I read this binding more it's seems wrong
>
> Today we do not use yet the watchdog framework which we will so we need to
> use the same binding as the watchdog framework

What is the binding of the heartbeat value in the watchdog framework?
I didn't found any binding of the heartbeat in other dt enabled drivers.

> So when we switch the DT will not change


Best regards
Jean-Christophe PLAGNIOL-VILLARD Sept. 27, 2012, 5:19 p.m. UTC | #3
On 16:59 Thu 27 Sep     , Fabio Porcedda wrote:
> On Thu, Sep 27, 2012 at 7:56 AM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 12:25 Tue 25 Sep     , 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..5947929
> >> --- /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:
> >> +- heartbeat: contains the watchdog heartbeats in seconds.
> > more I read this binding more it's seems wrong
> >
> > Today we do not use yet the watchdog framework which we will so we need to
> > use the same binding as the watchdog framework
> 
> What is the binding of the heartbeat value in the watchdog framework?
> I didn't found any binding of the heartbeat in other dt enabled drivers.
when you read the new watchdog framework cf watchdog_dev.c and watchdog_core.c

you will see that we can set a timeout which have the same meaning as
heartbeat

the key point here is you try to solve the DT the wrong way -> drivers
specific

the binding need to be framwork specific os here we need to do the binding of
the watchdog framework first and then use it in the at91sam9 driver evenif we
do not switch yet. Because in the DT we describe the hw not a drvier
implementation

Best Regards,
J.
Fabio Porcedda Sept. 28, 2012, 4:20 p.m. UTC | #4
On Thu, Sep 27, 2012 at 7:19 PM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 16:59 Thu 27 Sep     , Fabio Porcedda wrote:
>> On Thu, Sep 27, 2012 at 7:56 AM, Jean-Christophe PLAGNIOL-VILLARD
>> <plagnioj@jcrosoft.com> wrote:
>> > On 12:25 Tue 25 Sep     , 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..5947929
>> >> --- /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:
>> >> +- heartbeat: contains the watchdog heartbeats in seconds.
>> > more I read this binding more it's seems wrong
>> >
>> > Today we do not use yet the watchdog framework which we will so we need to
>> > use the same binding as the watchdog framework
>>
>> What is the binding of the heartbeat value in the watchdog framework?
>> I didn't found any binding of the heartbeat in other dt enabled drivers.
> when you read the new watchdog framework cf watchdog_dev.c and watchdog_core.c
>
> you will see that we can set a timeout which have the same meaning as
> heartbeat
>
> the key point here is you try to solve the DT the wrong way -> drivers
> specific
>
> the binding need to be framwork specific os here we need to do the binding of
> the watchdog framework first and then use it in the at91sam9 driver evenif we
> do not switch yet. Because in the DT we describe the hw not a drvier
> implementation

Ok i will try to send an updated patch following your advice.

> Best Regards,
> J.


Best regards
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt b/Documentation/devicetree/bindings/watchdog/atmel-wdt.txt
new file mode 100644
index 0000000..5947929
--- /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:
+- heartbeat: contains the watchdog heartbeats in seconds.
+
+Example:
+
+	watchdog@fffffd40 {
+		compatible = "atmel,at91sam9260-wdt";
+		reg = <0xfffffd40 0x10>;
+		heartbeat = <10>;
+	};
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index 05e1be8..84658d7 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, "heartbeat", &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),
 	},
 };