[PATCH 0/3] omap3/omap4: add device tree support for wdt

[PATCH 0/3] omap3/omap4: add device tree support for wdt

[jgq516]

From: Xiao Jiang <jgq516@gmail.com>

This series can be applied to dt branch of linux-omap tree.

Since omap24xx series has different wdt base addr (omap2420: 0x48022000 and
omap2430: 0x49016000) per commit 2817142f31bfbf26c216bf4f9192540c81b2d071, so
I don't add wdt node in omap2.dtsi just like omap3 and omap4, maybe different
dts files are needed for omap2420 and omap2430.

Tested with omap4430 blaze board.

Xiao Jiang (3):
  arm/dts: add wdt node for omap3 and omap4
  OMAP: wdt: add device tree support
  watchdog: omap_wdt: add device tree support

 arch/arm/boot/dts/omap3.dtsi  |    5 +++++
 arch/arm/boot/dts/omap4.dtsi  |    5 +++++
 arch/arm/mach-omap2/devices.c |    2 +-
 drivers/watchdog/omap_wdt.c   |    8 ++++++++
 4 files changed, 19 insertions(+), 1 deletions(-)

-- 
1.7.3

[PATCH 1/3] arm/dts: add wdt node for omap3 and omap4

[jgq516]

From: Xiao Jiang <jgq516@gmail.com>

Add wdt node to support dt.

Signed-off-by: Xiao Jiang <jgq516@gmail.com>
---
 arch/arm/boot/dts/omap3.dtsi |    5 +++++
 arch/arm/boot/dts/omap4.dtsi |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 99474fa..039df5c 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -215,5 +215,10 @@
 			compatible = "ti,omap3-hsmmc";
 			ti,hwmods = "mmc3";
 		};
+
+		wdt: wdt@48314000{
+			compatible = "ti,omap3-wdt";
+			ti,hwmods = "wd_timer2";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 359c497..d01403d 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -272,5 +272,10 @@
 			ti,hwmods = "mmc5";
 			ti,needs-special-reset;
 		};
+
+		wdt: wdt@4a314000{
+			compatible = "ti,omap4-wdt";
+			ti,hwmods = "wd_timer2";
+		};
 	};
 };
-- 
1.7.3

[PATCH 2/3] OMAP: avoid build wdt platform device if with dt support

[jgq516]

From: Xiao Jiang <jgq516@gmail.com>

If provided dt support, then skip add wdt platform device as usual.

Signed-off-by: Xiao Jiang <jgq516@gmail.com>
---
 arch/arm/mach-omap2/devices.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index ae62ece..80d7e3f 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -759,7 +759,7 @@ static int __init omap_init_wdt(void)
 	char *oh_name = "wd_timer2";
 	char *dev_name = "omap_wdt";
 
-	if (!cpu_class_is_omap2())
+	if (!cpu_class_is_omap2() || of_have_populated_dt())
 		return 0;
 
 	oh = omap_hwmod_lookup(oh_name);
-- 
1.7.3

[PATCH 3/3] watchdog: omap_wdt: add device tree support

[jgq516]

From: Xiao Jiang <jgq516@gmail.com>

Add device table for omap_wdt to support dt.

Signed-off-by: Xiao Jiang <jgq516@gmail.com>
---
 drivers/watchdog/omap_wdt.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 8285d65..d98c615 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -430,6 +430,13 @@ static int omap_wdt_resume(struct platform_device *pdev)
 #define	omap_wdt_resume		NULL
 #endif
 
+static const struct of_device_id omap_wdt_of_match[] = {
+	{ .compatible = "ti,omap3-wdt", },
+	{ .compatible = "ti,omap4-wdt", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
+
 static struct platform_driver omap_wdt_driver = {
 	.probe		= omap_wdt_probe,
 	.remove		= __devexit_p(omap_wdt_remove),
@@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "omap_wdt",
+		.of_match_table = omap_wdt_of_match,
 	},
 };
 
-- 
1.7.3

Re: [PATCH 0/3] omap3/omap4: add device tree support for wdt

[Jon Hunter]

Hi Xiao Jiang,

On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
> From: Xiao Jiang <jgq516@gmail.com>
> 
> This series can be applied to dt branch of linux-omap tree.

Thanks for sending this!

> Since omap24xx series has different wdt base addr (omap2420: 0x48022000 and
> omap2430: 0x49016000) per commit 2817142f31bfbf26c216bf4f9192540c81b2d071, so
> I don't add wdt node in omap2.dtsi just like omap3 and omap4, maybe different
> dts files are needed for omap2420 and omap2430.

Good point. I am wondering if we can simple drop the address from the
wdt2 node for omap2. It is not really being used. May be Benoit can comment.

Cheers
Jon

> Tested with omap4430 blaze board.
> 
> Xiao Jiang (3):
>   arm/dts: add wdt node for omap3 and omap4
>   OMAP: wdt: add device tree support
>   watchdog: omap_wdt: add device tree support
> 
>  arch/arm/boot/dts/omap3.dtsi  |    5 +++++
>  arch/arm/boot/dts/omap4.dtsi  |    5 +++++
>  arch/arm/mach-omap2/devices.c |    2 +-
>  drivers/watchdog/omap_wdt.c   |    8 ++++++++
>  4 files changed, 19 insertions(+), 1 deletions(-)
> 

Re: [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4

[Jon Hunter]

On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
> From: Xiao Jiang <jgq516@gmail.com>
> 
> Add wdt node to support dt.
> 
> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
> ---
>  arch/arm/boot/dts/omap3.dtsi |    5 +++++
>  arch/arm/boot/dts/omap4.dtsi |    5 +++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 99474fa..039df5c 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -215,5 +215,10 @@
>  			compatible = "ti,omap3-hsmmc";
>  			ti,hwmods = "mmc3";
>  		};
> +
> +		wdt: wdt@48314000{

Minor comment, may be call this wdt2 as this is watchdog timer 2. Also
ass a space between the address and the "{".

> +			compatible = "ti,omap3-wdt";
> +			ti,hwmods = "wd_timer2";
> +		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 359c497..d01403d 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -272,5 +272,10 @@
>  			ti,hwmods = "mmc5";
>  			ti,needs-special-reset;
>  		};
> +
> +		wdt: wdt@4a314000{

Same as above.

> +			compatible = "ti,omap4-wdt";
> +			ti,hwmods = "wd_timer2";
> +		};
>  	};
>  };

Also we should add some documentation to describe the binding. Here it
is very simple, but nonetheless we should have something as we have for
gpio [1].

Cheers
Jon

[1]
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/devicetree/bindings/gpio/gpio-omap.txt;h=bff51a2fee1eae7d387b40ea913e04782554bf68;hb=HEAD

Re: [PATCH 2/3] OMAP: avoid build wdt platform device if with dt support

[Jon Hunter]

On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
> From: Xiao Jiang <jgq516@gmail.com>
> 
> If provided dt support, then skip add wdt platform device as usual.
> 
> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
> ---
>  arch/arm/mach-omap2/devices.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index ae62ece..80d7e3f 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -759,7 +759,7 @@ static int __init omap_init_wdt(void)
>  	char *oh_name = "wd_timer2";
>  	char *dev_name = "omap_wdt";
>  
> -	if (!cpu_class_is_omap2())
> +	if (!cpu_class_is_omap2() || of_have_populated_dt())
>  		return 0;
>  
>  	oh = omap_hwmod_lookup(oh_name);

Reviewed-by: Jon Hunter <jon-hunter@ti.com>

Cheers
Jon

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Jon Hunter]

On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
> From: Xiao Jiang <jgq516@gmail.com>
> 
> Add device table for omap_wdt to support dt.
> 
> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
> ---
>  drivers/watchdog/omap_wdt.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
> index 8285d65..d98c615 100644
> --- a/drivers/watchdog/omap_wdt.c
> +++ b/drivers/watchdog/omap_wdt.c
> @@ -430,6 +430,13 @@ static int omap_wdt_resume(struct platform_device *pdev)
>  #define	omap_wdt_resume		NULL
>  #endif
>  
> +static const struct of_device_id omap_wdt_of_match[] = {
> +	{ .compatible = "ti,omap3-wdt", },
> +	{ .compatible = "ti,omap4-wdt", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
> +
>  static struct platform_driver omap_wdt_driver = {
>  	.probe		= omap_wdt_probe,
>  	.remove		= __devexit_p(omap_wdt_remove),
> @@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
>  	.driver		= {
>  		.owner	= THIS_MODULE,
>  		.name	= "omap_wdt",
> +		.of_match_table = omap_wdt_of_match,
>  	},
>  };
>  

I think we need to add some code to the probe function that calls
of_match_device() and ensures we find a match. For example ...

	if (of_have_populated_dt())
		if (!of_match_device(omap_wdt_of_match, &pdev->dev))
			return -EINVAL;

Cheers
Jon

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Xiao Jiang]

Jon Hunter wrote:
> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>   
>> From: Xiao Jiang <jgq516@gmail.com>
>>
>> Add device table for omap_wdt to support dt.
>>
>> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
>> ---
>>  drivers/watchdog/omap_wdt.c |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>> index 8285d65..d98c615 100644
>> --- a/drivers/watchdog/omap_wdt.c
>> +++ b/drivers/watchdog/omap_wdt.c
>> @@ -430,6 +430,13 @@ static int omap_wdt_resume(struct platform_device *pdev)
>>  #define	omap_wdt_resume		NULL
>>  #endif
>>  
>> +static const struct of_device_id omap_wdt_of_match[] = {
>> +	{ .compatible = "ti,omap3-wdt", },
>> +	{ .compatible = "ti,omap4-wdt", },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
>> +
>>  static struct platform_driver omap_wdt_driver = {
>>  	.probe		= omap_wdt_probe,
>>  	.remove		= __devexit_p(omap_wdt_remove),
>> @@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
>>  	.driver		= {
>>  		.owner	= THIS_MODULE,
>>  		.name	= "omap_wdt",
>> +		.of_match_table = omap_wdt_of_match,
>>  	},
>>  };
>>  
>>     
>
> I think we need to add some code to the probe function that calls
> of_match_device() and ensures we find a match. For example ...
>
> 	if (of_have_populated_dt())
> 		if (!of_match_device(omap_wdt_of_match, &pdev->dev))
> 			return -EINVAL;
>
>   
Will add it in v2, thanks for suggestion.

Regards,
Xiao
> Cheers
> Jon
>   

Re: [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4

[Xiao Jiang]

Jon Hunter wrote:
> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>   
>> From: Xiao Jiang <jgq516@gmail.com>
>>
>> Add wdt node to support dt.
>>
>> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
>> ---
>>  arch/arm/boot/dts/omap3.dtsi |    5 +++++
>>  arch/arm/boot/dts/omap4.dtsi |    5 +++++
>>  2 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>> index 99474fa..039df5c 100644
>> --- a/arch/arm/boot/dts/omap3.dtsi
>> +++ b/arch/arm/boot/dts/omap3.dtsi
>> @@ -215,5 +215,10 @@
>>  			compatible = "ti,omap3-hsmmc";
>>  			ti,hwmods = "mmc3";
>>  		};
>> +
>> +		wdt: wdt@48314000{
>>     
>
> Minor comment, may be call this wdt2 as this is watchdog timer 2. Also
> ass a space between the address and the "{".
>
>   
Ok, will change it to "wdt2: wdt@48314000 {".
>> +			compatible = "ti,omap3-wdt";
>> +			ti,hwmods = "wd_timer2";
>> +		};
>>  	};
>>  };
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 359c497..d01403d 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -272,5 +272,10 @@
>>  			ti,hwmods = "mmc5";
>>  			ti,needs-special-reset;
>>  		};
>> +
>> +		wdt: wdt@4a314000{
>>     
>
> Same as above.
>
>   
Ditto.
>> +			compatible = "ti,omap4-wdt";
>> +			ti,hwmods = "wd_timer2";
>> +		};
>>  	};
>>  };
>>     
>
> Also we should add some documentation to describe the binding. Here it
> is very simple, but nonetheless we should have something as we have for
> gpio [1].
>
>   
Thanks for reminding, how about below patch?

diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt 
b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
new file mode 100644
index 0000000..4272d06
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
@@ -0,0 +1,15 @@
+TI Watchdog Timer (WDT) Controller for OMAP
+
+Required properties:
+- compatible:
+  - "ti,omap2-wdt" for OMAP2
+  - "ti,omap3-wdt" for OMAP3
+  - "ti,omap4-wdt" for OMAP4
+- ti,hwmods: Name of the hwmod associated to the WDT
+
+Examples:
+
+wdt2: wdt@73f98000 {
+       compatible = "ti,omap4-wdt";
+       ti,hwmods = "wd_timer2";
+};

Regards,
Xiao
> Cheers
> Jon
>
> [1]
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/devicetree/bindings/gpio/gpio-omap.txt;h=bff51a2fee1eae7d387b40ea913e04782554bf68;hb=HEAD
>   

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Cousson, Benoit]

On 5/30/2012 5:18 AM, Xiao Jiang wrote:
> Jon Hunter wrote:
>> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>>> From: Xiao Jiang <jgq516@gmail.com>
>>>
>>> Add device table for omap_wdt to support dt.
>>>
>>> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
>>> ---
>>> drivers/watchdog/omap_wdt.c | 8 ++++++++
>>> 1 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>>> index 8285d65..d98c615 100644
>>> --- a/drivers/watchdog/omap_wdt.c
>>> +++ b/drivers/watchdog/omap_wdt.c
>>> @@ -430,6 +430,13 @@ static int omap_wdt_resume(struct
>>> platform_device *pdev)
>>> #define omap_wdt_resume NULL
>>> #endif
>>>
>>> +static const struct of_device_id omap_wdt_of_match[] = {
>>> + { .compatible = "ti,omap3-wdt", },
>>> + { .compatible = "ti,omap4-wdt", },

If there is no difference between the OMAP3 and the OMAP4 WDT IP, just 
add one entry "ti,omap3-wdt". And then in the OMAP4 DTS you will just 
put : compatible = "ti,omap3-wdt"; or compatible =  "ti,omap4-wdt", 
"ti,omap3-wdt";
I'm still a little bit confused about the real need for the 
"ti,omap4-wdt: entry, but it seems to be the way to do it in PPC.

>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
>>> +
>>> static struct platform_driver omap_wdt_driver = {
>>> .probe = omap_wdt_probe,
>>> .remove = __devexit_p(omap_wdt_remove),
>>> @@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
>>> .driver = {
>>> .owner = THIS_MODULE,
>>> .name = "omap_wdt",
>>> + .of_match_table = omap_wdt_of_match,
>>> },
>>> };
>>>
>>
>> I think we need to add some code to the probe function that calls
>> of_match_device() and ensures we find a match. For example ...
>>
>> if (of_have_populated_dt())
>> if (!of_match_device(omap_wdt_of_match, &pdev->dev))
>> return -EINVAL;
>>
> Will add it in v2, thanks for suggestion.

No, in fact this is not needed. We need that mainly when several 
instances can match the same driver and thus we select the proper one 
using the of_match_device. Otherwise, just check is the device_node is 
there.

In that case, the driver does not even care about any DT node so there 
is no need to add extra code for that. Keep it simple.

Regards,
Benoit

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Xiao Jiang]

Cousson, Benoit wrote:
> On 5/30/2012 5:18 AM, Xiao Jiang wrote:
>> Jon Hunter wrote:
>>> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>>>> From: Xiao Jiang <jgq516@gmail.com>
>>>>
>>>> Add device table for omap_wdt to support dt.
>>>>
>>>> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
>>>> ---
>>>> drivers/watchdog/omap_wdt.c | 8 ++++++++
>>>> 1 files changed, 8 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>>>> index 8285d65..d98c615 100644
>>>> --- a/drivers/watchdog/omap_wdt.c
>>>> +++ b/drivers/watchdog/omap_wdt.c
>>>> @@ -430,6 +430,13 @@ static int omap_wdt_resume(struct
>>>> platform_device *pdev)
>>>> #define omap_wdt_resume NULL
>>>> #endif
>>>>
>>>> +static const struct of_device_id omap_wdt_of_match[] = {
>>>> + { .compatible = "ti,omap3-wdt", },
>>>> + { .compatible = "ti,omap4-wdt", },
>
> If there is no difference between the OMAP3 and the OMAP4 WDT IP, just 
> add one entry "ti,omap3-wdt". And then in the OMAP4 DTS you will just 
> put : compatible = "ti,omap3-wdt"; or compatible =  "ti,omap4-wdt", 
> "ti,omap3-wdt";
> I'm still a little bit confused about the real need for the 
> "ti,omap4-wdt: entry, but it seems to be the way to do it in PPC.
I believe OMAP2, OMAP3 and OMAP4 share the same IP, so how about use 
"ti, omap2-wdt"? and other dts files
put compatible like "ti,omap4-wdt", "ti,omap2-wdt" and "ti,omap4-wdt", 
"ti,omap2-wdt".
>
>>>> + {},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
>>>> +
>>>> static struct platform_driver omap_wdt_driver = {
>>>> .probe = omap_wdt_probe,
>>>> .remove = __devexit_p(omap_wdt_remove),
>>>> @@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
>>>> .driver = {
>>>> .owner = THIS_MODULE,
>>>> .name = "omap_wdt",
>>>> + .of_match_table = omap_wdt_of_match,
>>>> },
>>>> };
>>>>
>>>
>>> I think we need to add some code to the probe function that calls
>>> of_match_device() and ensures we find a match. For example ...
>>>
>>> if (of_have_populated_dt())
>>> if (!of_match_device(omap_wdt_of_match, &pdev->dev))
>>> return -EINVAL;
>>>
>> Will add it in v2, thanks for suggestion.
>
> No, in fact this is not needed. We need that mainly when several 
> instances can match the same driver and thus we select the proper one 
> using the of_match_device. Otherwise, just check is the device_node is 
> there.
>
> In that case, the driver does not even care about any DT node so there 
> is no need to add extra code for that. Keep it simple.
>
Thanks for elaborating, simple is good for this one.

Regards,
Xiao
> Regards,
> Benoit

Re: [PATCH 0/3] omap3/omap4: add device tree support for wdt

[Xiao Jiang]

Jon Hunter wrote:
> Hi Xiao Jiang,
>
> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>   
>> From: Xiao Jiang <jgq516@gmail.com>
>>
>> This series can be applied to dt branch of linux-omap tree.
>>     
>
> Thanks for sending this!
>
>   
>> Since omap24xx series has different wdt base addr (omap2420: 0x48022000 and
>> omap2430: 0x49016000) per commit 2817142f31bfbf26c216bf4f9192540c81b2d071, so
>> I don't add wdt node in omap2.dtsi just like omap3 and omap4, maybe different
>> dts files are needed for omap2420 and omap2430.
>>     
>
> Good point. I am wondering if we can simple drop the address from the
> wdt2 node for omap2. It is not really being used. May be Benoit can comment.
>
>   
Hmm, I think the address doesn't need anymore since the related hw_mod 
has the right addr as follows.

static struct omap_hwmod_addr_space omap2420_wd_timer2_addrs[] = {
        {  
                .pa_start       = 0x48022000,
                .pa_end         = 0x4802207f,
                .flags          = ADDR_TYPE_RT
        }, 
        { }
};

static struct omap_hwmod_addr_space omap2430_wd_timer2_addrs[] = {
        {  
                .pa_start       = 0x49016000,
                .pa_end         = 0x4901607f,
                .flags          = ADDR_TYPE_RT
        }, 
        { }
};

static struct omap_hwmod_addr_space omap3xxx_wd_timer2_addrs[] = {
        {   
                .pa_start       = 0x48314000,
                .pa_end         = 0x4831407f,
                .flags          = ADDR_TYPE_RT
        },  
        { } 
};

static struct omap_hwmod_addr_space omap44xx_wd_timer2_addrs[] = {
        {   
                .pa_start       = 0x4a313000,
                .pa_end         = 0x4a31407f,
                .flags          = ADDR_TYPE_RT
        },  
        { } 
};

Regards,
Xiao
> Cheers
> Jon

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Xiao Jiang]

>> If there is no difference between the OMAP3 and the OMAP4 WDT IP, 
>> just add one entry "ti,omap3-wdt". And then in the OMAP4 DTS you will 
>> just put : compatible = "ti,omap3-wdt"; or compatible =  
>> "ti,omap4-wdt", "ti,omap3-wdt";
>> I'm still a little bit confused about the real need for the 
>> "ti,omap4-wdt: entry, but it seems to be the way to do it in PPC.
> I believe OMAP2, OMAP3 and OMAP4 share the same IP, so how about use 
> "ti, omap2-wdt"? and other dts files
> put compatible like "ti,omap4-wdt", "ti,omap2-wdt" and "ti,omap4-wdt", 
> "ti,omap2-wdt".
>
Typo, should be "ti,omap3-wdt", "ti,omap2-wdt" and "ti,omap4-wdt", 
"ti,omap2-wdt".

Regards,
Xiao

Re: [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4

[Jon Hunter]

On 05/29/2012 10:19 PM, Xiao Jiang wrote:
> Jon Hunter wrote:
>> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>>  
>>> From: Xiao Jiang <jgq516@gmail.com>
>>>
>>> Add wdt node to support dt.
>>>
>>> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
>>> ---
>>>  arch/arm/boot/dts/omap3.dtsi |    5 +++++
>>>  arch/arm/boot/dts/omap4.dtsi |    5 +++++
>>>  2 files changed, 10 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>>> index 99474fa..039df5c 100644
>>> --- a/arch/arm/boot/dts/omap3.dtsi
>>> +++ b/arch/arm/boot/dts/omap3.dtsi
>>> @@ -215,5 +215,10 @@
>>>              compatible = "ti,omap3-hsmmc";
>>>              ti,hwmods = "mmc3";
>>>          };
>>> +
>>> +        wdt: wdt@48314000{
>>>     
>>
>> Minor comment, may be call this wdt2 as this is watchdog timer 2. Also
>> ass a space between the address and the "{".
>>
>>   
> Ok, will change it to "wdt2: wdt@48314000 {".
>>> +            compatible = "ti,omap3-wdt";
>>> +            ti,hwmods = "wd_timer2";
>>> +        };
>>>      };
>>>  };
>>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>>> index 359c497..d01403d 100644
>>> --- a/arch/arm/boot/dts/omap4.dtsi
>>> +++ b/arch/arm/boot/dts/omap4.dtsi
>>> @@ -272,5 +272,10 @@
>>>              ti,hwmods = "mmc5";
>>>              ti,needs-special-reset;
>>>          };
>>> +
>>> +        wdt: wdt@4a314000{
>>>     
>>
>> Same as above.
>>
>>   
> Ditto.
>>> +            compatible = "ti,omap4-wdt";
>>> +            ti,hwmods = "wd_timer2";
>>> +        };
>>>      };
>>>  };
>>>     
>>
>> Also we should add some documentation to describe the binding. Here it
>> is very simple, but nonetheless we should have something as we have for
>> gpio [1].
>>
>>   
> Thanks for reminding, how about below patch?
> 
> diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
> b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
> new file mode 100644
> index 0000000..4272d06
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
> @@ -0,0 +1,15 @@
> +TI Watchdog Timer (WDT) Controller for OMAP
> +
> +Required properties:
> +- compatible:
> +  - "ti,omap2-wdt" for OMAP2
> +  - "ti,omap3-wdt" for OMAP3
> +  - "ti,omap4-wdt" for OMAP4
> +- ti,hwmods: Name of the hwmod associated to the WDT
> +
> +Examples:
> +
> +wdt2: wdt@73f98000 {
> +       compatible = "ti,omap4-wdt";
> +       ti,hwmods = "wd_timer2";
> +};

Yes looks good. Thanks! Minor nit-pick in the example I would just copy
the omap4 node completely with the actual omap4 address :-)

Cheers
Jon

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Jon Hunter]

Hi Benoit,

On 05/30/2012 02:54 AM, Cousson, Benoit wrote:
> On 5/30/2012 5:18 AM, Xiao Jiang wrote:
>> Jon Hunter wrote:
>>> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>>>> From: Xiao Jiang <jgq516@gmail.com>
>>>>
>>>> Add device table for omap_wdt to support dt.
>>>>
>>>> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
>>>> ---
>>>> drivers/watchdog/omap_wdt.c | 8 ++++++++
>>>> 1 files changed, 8 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>>>> index 8285d65..d98c615 100644
>>>> --- a/drivers/watchdog/omap_wdt.c
>>>> +++ b/drivers/watchdog/omap_wdt.c
>>>> @@ -430,6 +430,13 @@ static int omap_wdt_resume(struct
>>>> platform_device *pdev)
>>>> #define omap_wdt_resume NULL
>>>> #endif
>>>>
>>>> +static const struct of_device_id omap_wdt_of_match[] = {
>>>> + { .compatible = "ti,omap3-wdt", },
>>>> + { .compatible = "ti,omap4-wdt", },
> 
> If there is no difference between the OMAP3 and the OMAP4 WDT IP, just
> add one entry "ti,omap3-wdt". And then in the OMAP4 DTS you will just
> put : compatible = "ti,omap3-wdt"; or compatible =  "ti,omap4-wdt",
> "ti,omap3-wdt";

Hmmm ... comparing the omap3 and omap4 wdt registers there are some
differences. omap4 seems to have more registers than omap3. May be we
are not using these right now, but from a register perspective the wdt
in omap2, omap3 and omap4 appear to be slightly different. The revision
ID register on omap3 and omap4 have different values too.

I guess from a driver perspective there is no difference, but it seemed
to me that the IP is not completely the same.

> I'm still a little bit confused about the real need for the
> "ti,omap4-wdt: entry, but it seems to be the way to do it in PPC.
> 
>>>> + {},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
>>>> +
>>>> static struct platform_driver omap_wdt_driver = {
>>>> .probe = omap_wdt_probe,
>>>> .remove = __devexit_p(omap_wdt_remove),
>>>> @@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
>>>> .driver = {
>>>> .owner = THIS_MODULE,
>>>> .name = "omap_wdt",
>>>> + .of_match_table = omap_wdt_of_match,
>>>> },
>>>> };
>>>>
>>>
>>> I think we need to add some code to the probe function that calls
>>> of_match_device() and ensures we find a match. For example ...
>>>
>>> if (of_have_populated_dt())
>>> if (!of_match_device(omap_wdt_of_match, &pdev->dev))
>>> return -EINVAL;
>>>
>> Will add it in v2, thanks for suggestion.
> 
> No, in fact this is not needed. We need that mainly when several
> instances can match the same driver and thus we select the proper one
> using the of_match_device. Otherwise, just check is the device_node is
> there.
> 
> In that case, the driver does not even care about any DT node so there
> is no need to add extra code for that. Keep it simple.

Ok. So are you saying get rid of the match table altogether? In other
words, drop this patch?

I agree that it does not really do anything today, but I did not know if
in the future you were planning to pass things like, register addresses,
via DT.

Cheers
Jon

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Cousson, Benoit]

Hi Jon,

On 5/30/2012 5:03 PM, Jon Hunter wrote:
> Hi Benoit,
>
> On 05/30/2012 02:54 AM, Cousson, Benoit wrote:
>> On 5/30/2012 5:18 AM, Xiao Jiang wrote:
>>> Jon Hunter wrote:
>>>> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>>>>> From: Xiao Jiang<jgq516@gmail.com>
>>>>>
>>>>> Add device table for omap_wdt to support dt.
>>>>>
>>>>> Signed-off-by: Xiao Jiang<jgq516@gmail.com>
>>>>> ---
>>>>> drivers/watchdog/omap_wdt.c | 8 ++++++++
>>>>> 1 files changed, 8 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>>>>> index 8285d65..d98c615 100644
>>>>> --- a/drivers/watchdog/omap_wdt.c
>>>>> +++ b/drivers/watchdog/omap_wdt.c
>>>>> @@ -430,6 +430,13 @@ static int omap_wdt_resume(struct
>>>>> platform_device *pdev)
>>>>> #define omap_wdt_resume NULL
>>>>> #endif
>>>>>
>>>>> +static const struct of_device_id omap_wdt_of_match[] = {
>>>>> + { .compatible = "ti,omap3-wdt", },
>>>>> + { .compatible = "ti,omap4-wdt", },
>>
>> If there is no difference between the OMAP3 and the OMAP4 WDT IP, just
>> add one entry "ti,omap3-wdt". And then in the OMAP4 DTS you will just
>> put : compatible = "ti,omap3-wdt"; or compatible =  "ti,omap4-wdt",
>> "ti,omap3-wdt";
>
> Hmmm ... comparing the omap3 and omap4 wdt registers there are some
> differences. omap4 seems to have more registers than omap3. May be we
> are not using these right now, but from a register perspective the wdt
> in omap2, omap3 and omap4 appear to be slightly different. The revision
> ID register on omap3 and omap4 have different values too.
>
> I guess from a driver perspective there is no difference, but it seemed
> to me that the IP is not completely the same.

Well, in that case, and assuming that there is no proper HW_REVISION 
information to detect the IP difference, the proper compatible entries 
will indeed have to be used.

>
>> I'm still a little bit confused about the real need for the
>> "ti,omap4-wdt: entry, but it seems to be the way to do it in PPC.
>>
>>>>> + {},
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
>>>>> +
>>>>> static struct platform_driver omap_wdt_driver = {
>>>>> .probe = omap_wdt_probe,
>>>>> .remove = __devexit_p(omap_wdt_remove),
>>>>> @@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
>>>>> .driver = {
>>>>> .owner = THIS_MODULE,
>>>>> .name = "omap_wdt",
>>>>> + .of_match_table = omap_wdt_of_match,
>>>>> },
>>>>> };
>>>>>
>>>>
>>>> I think we need to add some code to the probe function that calls
>>>> of_match_device() and ensures we find a match. For example ...
>>>>
>>>> if (of_have_populated_dt())
>>>> if (!of_match_device(omap_wdt_of_match,&pdev->dev))
>>>> return -EINVAL;
>>>>
>>> Will add it in v2, thanks for suggestion.
>>
>> No, in fact this is not needed. We need that mainly when several
>> instances can match the same driver and thus we select the proper one
>> using the of_match_device. Otherwise, just check is the device_node is
>> there.
>>
>> In that case, the driver does not even care about any DT node so there
>> is no need to add extra code for that. Keep it simple.
>
> Ok. So are you saying get rid of the match table altogether? In other
> words, drop this patch?

No, the match table is used by the LDM to find the proper driver to be 
bound to a device. So we do need it. But we do not have to use the 
of_match_device if we do not want to get the entry in the device table.

> I agree that it does not really do anything today, but I did not know if
> in the future you were planning to pass things like, register addresses,
> via DT.

Well, yes we will have to, otherwise people will keep complaining that 
our DTS sucks and are not compliant with the DTS standards :-)

Regards,
Benoit

Re: [PATCH 3/3] watchdog: omap_wdt: add device tree support

[Jon Hunter]

Hi Benoit,

On 05/30/2012 10:30 AM, Cousson, Benoit wrote:
> Hi Jon,
> 
> On 5/30/2012 5:03 PM, Jon Hunter wrote:
>> Hi Benoit,
>>
>> On 05/30/2012 02:54 AM, Cousson, Benoit wrote:
>>> On 5/30/2012 5:18 AM, Xiao Jiang wrote:
>>>> Jon Hunter wrote:
>>>>> On 05/25/2012 05:42 AM, jgq516@gmail.com wrote:
>>>>>> From: Xiao Jiang<jgq516@gmail.com>
>>>>>>
>>>>>> Add device table for omap_wdt to support dt.
>>>>>>
>>>>>> Signed-off-by: Xiao Jiang<jgq516@gmail.com>
>>>>>> ---
>>>>>> drivers/watchdog/omap_wdt.c | 8 ++++++++
>>>>>> 1 files changed, 8 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/watchdog/omap_wdt.c
>>>>>> b/drivers/watchdog/omap_wdt.c
>>>>>> index 8285d65..d98c615 100644
>>>>>> --- a/drivers/watchdog/omap_wdt.c
>>>>>> +++ b/drivers/watchdog/omap_wdt.c
>>>>>> @@ -430,6 +430,13 @@ static int omap_wdt_resume(struct
>>>>>> platform_device *pdev)
>>>>>> #define omap_wdt_resume NULL
>>>>>> #endif
>>>>>>
>>>>>> +static const struct of_device_id omap_wdt_of_match[] = {
>>>>>> + { .compatible = "ti,omap3-wdt", },
>>>>>> + { .compatible = "ti,omap4-wdt", },
>>>
>>> If there is no difference between the OMAP3 and the OMAP4 WDT IP, just
>>> add one entry "ti,omap3-wdt". And then in the OMAP4 DTS you will just
>>> put : compatible = "ti,omap3-wdt"; or compatible =  "ti,omap4-wdt",
>>> "ti,omap3-wdt";
>>
>> Hmmm ... comparing the omap3 and omap4 wdt registers there are some
>> differences. omap4 seems to have more registers than omap3. May be we
>> are not using these right now, but from a register perspective the wdt
>> in omap2, omap3 and omap4 appear to be slightly different. The revision
>> ID register on omap3 and omap4 have different values too.
>>
>> I guess from a driver perspective there is no difference, but it seemed
>> to me that the IP is not completely the same.
> 
> Well, in that case, and assuming that there is no proper HW_REVISION
> information to detect the IP difference, the proper compatible entries
> will indeed have to be used.

So looking at a 4460 and 3430, the WIDR register (IP revision) can be
used to distinguish between IP revisions. So it appears that we do have
proper HW REV info.

So may be I am not completely up to speed of the intent of the
compatible field. In other words, should this be used to indicate if the
IP is same/compatible or the driver is compatible or both. Technically
right now we could just have "ti-omap2-wdt" for all omap2+ devices as
the driver is compatible for all devices. However, technically, the IP
is not completely the same but it is compatible :-)

>>> I'm still a little bit confused about the real need for the
>>> "ti,omap4-wdt: entry, but it seems to be the way to do it in PPC.
>>>
>>>>>> + {},
>>>>>> +};
>>>>>> +MODULE_DEVICE_TABLE(of, omap_wdt_of_match);
>>>>>> +
>>>>>> static struct platform_driver omap_wdt_driver = {
>>>>>> .probe = omap_wdt_probe,
>>>>>> .remove = __devexit_p(omap_wdt_remove),
>>>>>> @@ -439,6 +446,7 @@ static struct platform_driver omap_wdt_driver = {
>>>>>> .driver = {
>>>>>> .owner = THIS_MODULE,
>>>>>> .name = "omap_wdt",
>>>>>> + .of_match_table = omap_wdt_of_match,
>>>>>> },
>>>>>> };
>>>>>>
>>>>>
>>>>> I think we need to add some code to the probe function that calls
>>>>> of_match_device() and ensures we find a match. For example ...
>>>>>
>>>>> if (of_have_populated_dt())
>>>>> if (!of_match_device(omap_wdt_of_match,&pdev->dev))
>>>>> return -EINVAL;
>>>>>
>>>> Will add it in v2, thanks for suggestion.
>>>
>>> No, in fact this is not needed. We need that mainly when several
>>> instances can match the same driver and thus we select the proper one
>>> using the of_match_device. Otherwise, just check is the device_node is
>>> there.
>>>
>>> In that case, the driver does not even care about any DT node so there
>>> is no need to add extra code for that. Keep it simple.
>>
>> Ok. So are you saying get rid of the match table altogether? In other
>> words, drop this patch?
> 
> No, the match table is used by the LDM to find the proper driver to be
> bound to a device. So we do need it. But we do not have to use the
> of_match_device if we do not want to get the entry in the device table.

Ok, thanks.

>> I agree that it does not really do anything today, but I did not know if
>> in the future you were planning to pass things like, register addresses,
>> via DT.
> 
> Well, yes we will have to, otherwise people will keep complaining that
> our DTS sucks and are not compliant with the DTS standards :-)

Ok.

Jon

Re: [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4

[Xiao Jiang]

Hi Jon and Benoit,
>> Thanks for reminding, how about below patch?
>>
>> diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>> b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>> new file mode 100644
>> index 0000000..4272d06
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>> @@ -0,0 +1,15 @@
>> +TI Watchdog Timer (WDT) Controller for OMAP
>> +
>> +Required properties:
>> +- compatible:
>> +  - "ti,omap2-wdt" for OMAP2
>> +  - "ti,omap3-wdt" for OMAP3
>> +  - "ti,omap4-wdt" for OMAP4
>> +- ti,hwmods: Name of the hwmod associated to the WDT
>> +
>> +Examples:
>> +
>> +wdt2: wdt@73f98000 {
>> +       compatible = "ti,omap4-wdt";
>> +       ti,hwmods = "wd_timer2";
>> +};
>>     
>
> Yes looks good. Thanks! Minor nit-pick in the example I would just copy
> the omap4 node completely with the actual omap4 address :-)
>
>   
Oops, wrong addr, :). Perhaps we can drop address as you said, since the 
right addresses are defined
in wd_timer2 hwmod (see [1]), and wdt also works without the address as 
follows.

diff --git a/arch/arm/boot/dts/omap2.dtsi b/arch/arm/boot/dts/omap2.dtsi
index f2ab4ea..0017bd8 100644
--- a/arch/arm/boot/dts/omap2.dtsi
+++ b/arch/arm/boot/dts/omap2.dtsi
@@ -63,5 +63,10 @@
                        ti,hwmods = "uart3";
                        clock-frequency = <48000000>;
                };
+
+               wdt2: wdt {
+                       compatible = "ti,omap2-wdt";
+                       ti,hwmods = "wd_timer2";
+               };
        };
 };
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 99474fa..dbf8a5b 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -215,5 +215,10 @@
                        compatible = "ti,omap3-hsmmc";
                        ti,hwmods = "mmc3";
                };
+
+               wdt2: wdt {
+                       compatible = "ti,omap3-wdt", "ti,omap2-wdt";
+                       ti,hwmods = "wd_timer2";
+               };
        };
 };
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 359c497..ce74e87 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -272,5 +272,10 @@
                        ti,hwmods = "mmc5";
                        ti,needs-special-reset;
                };
+
+               wdt2: wdt {
+                       compatible = "ti,omap4-wdt", "ti,omap2-wdt";
+                       ti,hwmods = "wd_timer2";
+               };
        };
 };

Infos for omap3:
# dmesg|grep Machine
<6>[    0.000000] Machine: Generic OMAP3 (Flattened Device Tree), model: 
TI OMAP3 EVM (OMAP3530, AM/DM37x)
# dmesg|grep omap_wdt_probe
<4>[    2.552825] in omap_wdt_probe: 299, res->start = 0x48314000

Infos for omap4:
root@localhost:/root> dmesg|grep Machine
[    0.000000] Machine: Generic OMAP4 (Flattened Device Tree), model: TI 
OMAP4 SDP board
root@localhost:/root> dmesg|grep omap_wdt_probe
[    1.687896] in omap_wdt_probe: 299, res->start = 0x4a314000

So can I drop the wdt addr from dts file? otherwise it is not feasible 
to add omap2 wdt node in omap2.dtsi
due to different addrs for omap2420 and omap2430.

Regards,
Xiao
[1] http://marc.info/?l=linux-omap&m=133836995026565&w=2
> Cheers
> Jon
>   

Re: [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4

[Jon Hunter]

On 05/31/2012 12:51 AM, Xiao Jiang wrote:
> Hi Jon and Benoit,
>>> Thanks for reminding, how about below patch?
>>>
>>> diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>>> b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>>> new file mode 100644
>>> index 0000000..4272d06
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>>> @@ -0,0 +1,15 @@
>>> +TI Watchdog Timer (WDT) Controller for OMAP
>>> +
>>> +Required properties:
>>> +- compatible:
>>> +  - "ti,omap2-wdt" for OMAP2
>>> +  - "ti,omap3-wdt" for OMAP3
>>> +  - "ti,omap4-wdt" for OMAP4
>>> +- ti,hwmods: Name of the hwmod associated to the WDT
>>> +
>>> +Examples:
>>> +
>>> +wdt2: wdt@73f98000 {
>>> +       compatible = "ti,omap4-wdt";
>>> +       ti,hwmods = "wd_timer2";
>>> +};
>>>     
>>
>> Yes looks good. Thanks! Minor nit-pick in the example I would just copy
>> the omap4 node completely with the actual omap4 address :-)
>>
>>   
> Oops, wrong addr, :). Perhaps we can drop address as you said, since the
> right addresses are defined
> in wd_timer2 hwmod (see [1]), and wdt also works without the address as
> follows.
> 
> diff --git a/arch/arm/boot/dts/omap2.dtsi b/arch/arm/boot/dts/omap2.dtsi
> index f2ab4ea..0017bd8 100644
> --- a/arch/arm/boot/dts/omap2.dtsi
> +++ b/arch/arm/boot/dts/omap2.dtsi
> @@ -63,5 +63,10 @@
>                        ti,hwmods = "uart3";
>                        clock-frequency = <48000000>;
>                };
> +
> +               wdt2: wdt {
> +                       compatible = "ti,omap2-wdt";
> +                       ti,hwmods = "wd_timer2";
> +               };
>        };
> };
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 99474fa..dbf8a5b 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -215,5 +215,10 @@
>                        compatible = "ti,omap3-hsmmc";
>                        ti,hwmods = "mmc3";
>                };
> +
> +               wdt2: wdt {
> +                       compatible = "ti,omap3-wdt", "ti,omap2-wdt";
> +                       ti,hwmods = "wd_timer2";
> +               };
>        };
> };
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 359c497..ce74e87 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -272,5 +272,10 @@
>                        ti,hwmods = "mmc5";
>                        ti,needs-special-reset;
>                };
> +
> +               wdt2: wdt {
> +                       compatible = "ti,omap4-wdt", "ti,omap2-wdt";
> +                       ti,hwmods = "wd_timer2";
> +               };
>        };
> };
> 
> Infos for omap3:
> # dmesg|grep Machine
> <6>[    0.000000] Machine: Generic OMAP3 (Flattened Device Tree), model:
> TI OMAP3 EVM (OMAP3530, AM/DM37x)
> # dmesg|grep omap_wdt_probe
> <4>[    2.552825] in omap_wdt_probe: 299, res->start = 0x48314000
> 
> Infos for omap4:
> root@localhost:/root> dmesg|grep Machine
> [    0.000000] Machine: Generic OMAP4 (Flattened Device Tree), model: TI
> OMAP4 SDP board
> root@localhost:/root> dmesg|grep omap_wdt_probe
> [    1.687896] in omap_wdt_probe: 299, res->start = 0x4a314000
> 
> So can I drop the wdt addr from dts file? otherwise it is not feasible
> to add omap2 wdt node in omap2.dtsi
> due to different addrs for omap2420 and omap2430.

Benoit, what is your preference here?

Cheers
Jon

Re: [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4

[Cousson, Benoit]

On 5/31/2012 4:55 PM, Jon Hunter wrote:
> On 05/31/2012 12:51 AM, Xiao Jiang wrote:
>> Hi Jon and Benoit,
>>>> Thanks for reminding, how about below patch?
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>>>> b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>>>> new file mode 100644
>>>> index 0000000..4272d06
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/watchdog/omap-wdt.txt
>>>> @@ -0,0 +1,15 @@
>>>> +TI Watchdog Timer (WDT) Controller for OMAP
>>>> +
>>>> +Required properties:
>>>> +- compatible:
>>>> +  - "ti,omap2-wdt" for OMAP2
>>>> +  - "ti,omap3-wdt" for OMAP3
>>>> +  - "ti,omap4-wdt" for OMAP4
>>>> +- ti,hwmods: Name of the hwmod associated to the WDT
>>>> +
>>>> +Examples:
>>>> +
>>>> +wdt2: wdt@73f98000 {
>>>> +       compatible = "ti,omap4-wdt";
>>>> +       ti,hwmods = "wd_timer2";
>>>> +};
>>>>
>>>
>>> Yes looks good. Thanks! Minor nit-pick in the example I would just copy
>>> the omap4 node completely with the actual omap4 address :-)
>>>
>>>
>> Oops, wrong addr, :). Perhaps we can drop address as you said, since the
>> right addresses are defined
>> in wd_timer2 hwmod (see [1]), and wdt also works without the address as
>> follows.
>>
>> diff --git a/arch/arm/boot/dts/omap2.dtsi b/arch/arm/boot/dts/omap2.dtsi
>> index f2ab4ea..0017bd8 100644
>> --- a/arch/arm/boot/dts/omap2.dtsi
>> +++ b/arch/arm/boot/dts/omap2.dtsi
>> @@ -63,5 +63,10 @@
>>                         ti,hwmods = "uart3";
>>                         clock-frequency =<48000000>;
>>                 };
>> +
>> +               wdt2: wdt {
>> +                       compatible = "ti,omap2-wdt";
>> +                       ti,hwmods = "wd_timer2";
>> +               };
>>         };
>> };
>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>> index 99474fa..dbf8a5b 100644
>> --- a/arch/arm/boot/dts/omap3.dtsi
>> +++ b/arch/arm/boot/dts/omap3.dtsi
>> @@ -215,5 +215,10 @@
>>                         compatible = "ti,omap3-hsmmc";
>>                         ti,hwmods = "mmc3";
>>                 };
>> +
>> +               wdt2: wdt {
>> +                       compatible = "ti,omap3-wdt", "ti,omap2-wdt";
>> +                       ti,hwmods = "wd_timer2";
>> +               };
>>         };
>> };
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 359c497..ce74e87 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -272,5 +272,10 @@
>>                         ti,hwmods = "mmc5";
>>                         ti,needs-special-reset;
>>                 };
>> +
>> +               wdt2: wdt {
>> +                       compatible = "ti,omap4-wdt", "ti,omap2-wdt";
>> +                       ti,hwmods = "wd_timer2";
>> +               };
>>         };
>> };
>>
>> Infos for omap3:
>> # dmesg|grep Machine
>> <6>[    0.000000] Machine: Generic OMAP3 (Flattened Device Tree), model:
>> TI OMAP3 EVM (OMAP3530, AM/DM37x)
>> # dmesg|grep omap_wdt_probe
>> <4>[    2.552825] in omap_wdt_probe: 299, res->start = 0x48314000
>>
>> Infos for omap4:
>> root@localhost:/root>  dmesg|grep Machine
>> [    0.000000] Machine: Generic OMAP4 (Flattened Device Tree), model: TI
>> OMAP4 SDP board
>> root@localhost:/root>  dmesg|grep omap_wdt_probe
>> [    1.687896] in omap_wdt_probe: 299, res->start = 0x4a314000
>>
>> So can I drop the wdt addr from dts file? otherwise it is not feasible
>> to add omap2 wdt node in omap2.dtsi
>> due to different addrs for omap2420 and omap2430.
>
> Benoit, what is your preference here?

Get rid of both omap2420 and 2430 :-)

The point is that only OMAP3 and OMAP4 are supposed to be migrated to DT 
for the moment.

If you do not have any OMAP2 board to test that, it is anyway safer to 
not touch the omap2.dtsi file.

If the 2 or 3 remaining users of OMAP2 boards want to have DT support, 
they'll be able to add that themselves.

Regards,
Benoit