2012-05-25 09:56:37

by Guoqing Jiang

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

From: Xiao Jiang <[email protected]>

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


2012-05-25 09:51:44

by Guoqing Jiang

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

From: Xiao Jiang <[email protected]>

Add device table for omap_wdt to support dt.

Signed-off-by: Xiao Jiang <[email protected]>
---
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

2012-05-25 09:51:43

by Guoqing Jiang

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

From: Xiao Jiang <[email protected]>

Add wdt node to support dt.

Signed-off-by: Xiao Jiang <[email protected]>
---
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

2012-05-25 09:51:42

by Guoqing Jiang

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

From: Xiao Jiang <[email protected]>

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

Signed-off-by: Xiao Jiang <[email protected]>
---
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

2012-05-29 17:47:37

by Jon Hunter

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

Hi Xiao Jiang,

On 05/25/2012 05:42 AM, [email protected] wrote:
> From: Xiao Jiang <[email protected]>
>
> 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(-)
>

2012-05-29 17:52:32

by Jon Hunter

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


On 05/25/2012 05:42 AM, [email protected] wrote:
> From: Xiao Jiang <[email protected]>
>
> Add wdt node to support dt.
>
> Signed-off-by: Xiao Jiang <[email protected]>
> ---
> 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

2012-05-29 17:53:56

by Jon Hunter

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



On 05/25/2012 05:42 AM, [email protected] wrote:
> From: Xiao Jiang <[email protected]>
>
> If provided dt support, then skip add wdt platform device as usual.
>
> Signed-off-by: Xiao Jiang <[email protected]>
> ---
> 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 <[email protected]>

Cheers
Jon

2012-05-29 18:06:17

by Jon Hunter

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


On 05/25/2012 05:42 AM, [email protected] wrote:
> From: Xiao Jiang <[email protected]>
>
> Add device table for omap_wdt to support dt.
>
> Signed-off-by: Xiao Jiang <[email protected]>
> ---
> 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

2012-05-30 02:26:17

by Guoqing Jiang

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

Jon Hunter wrote:
> On 05/25/2012 05:42 AM, [email protected] wrote:
>
>> From: Xiao Jiang <[email protected]>
>>
>> Add device table for omap_wdt to support dt.
>>
>> Signed-off-by: Xiao Jiang <[email protected]>
>> ---
>> 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
>

2012-05-30 02:27:44

by Guoqing Jiang

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

Jon Hunter wrote:
> On 05/25/2012 05:42 AM, [email protected] wrote:
>
>> From: Xiao Jiang <[email protected]>
>>
>> Add wdt node to support dt.
>>
>> Signed-off-by: Xiao Jiang <[email protected]>
>> ---
>> 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
>

2012-05-30 07:54:31

by Benoit Cousson

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

On 5/30/2012 5:18 AM, Xiao Jiang wrote:
> Jon Hunter wrote:
>> On 05/25/2012 05:42 AM, [email protected] wrote:
>>> From: Xiao Jiang <[email protected]>
>>>
>>> Add device table for omap_wdt to support dt.
>>>
>>> Signed-off-by: Xiao Jiang <[email protected]>
>>> ---
>>> 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

2012-05-30 09:22:22

by Guoqing Jiang

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

Cousson, Benoit wrote:
> On 5/30/2012 5:18 AM, Xiao Jiang wrote:
>> Jon Hunter wrote:
>>> On 05/25/2012 05:42 AM, [email protected] wrote:
>>>> From: Xiao Jiang <[email protected]>
>>>>
>>>> Add device table for omap_wdt to support dt.
>>>>
>>>> Signed-off-by: Xiao Jiang <[email protected]>
>>>> ---
>>>> 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

2012-05-30 09:22:46

by Guoqing Jiang

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

Jon Hunter wrote:
> Hi Xiao Jiang,
>
> On 05/25/2012 05:42 AM, [email protected] wrote:
>
>> From: Xiao Jiang <[email protected]>
>>
>> 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

2012-05-30 09:39:12

by Guoqing Jiang

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


>> 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

2012-05-30 14:43:50

by Jon Hunter

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


On 05/29/2012 10:19 PM, Xiao Jiang wrote:
> Jon Hunter wrote:
>> On 05/25/2012 05:42 AM, [email protected] wrote:
>>
>>> From: Xiao Jiang <[email protected]>
>>>
>>> Add wdt node to support dt.
>>>
>>> Signed-off-by: Xiao Jiang <[email protected]>
>>> ---
>>> 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

2012-05-30 15:03:56

by Jon Hunter

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

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, [email protected] wrote:
>>>> From: Xiao Jiang <[email protected]>
>>>>
>>>> Add device table for omap_wdt to support dt.
>>>>
>>>> Signed-off-by: Xiao Jiang <[email protected]>
>>>> ---
>>>> 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

2012-05-30 15:30:29

by Benoit Cousson

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

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, [email protected] wrote:
>>>>> From: Xiao Jiang<[email protected]>
>>>>>
>>>>> Add device table for omap_wdt to support dt.
>>>>>
>>>>> Signed-off-by: Xiao Jiang<[email protected]>
>>>>> ---
>>>>> 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

2012-05-30 16:13:06

by Jon Hunter

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

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, [email protected] wrote:
>>>>>> From: Xiao Jiang<[email protected]>
>>>>>>
>>>>>> Add device table for omap_wdt to support dt.
>>>>>>
>>>>>> Signed-off-by: Xiao Jiang<[email protected]>
>>>>>> ---
>>>>>> 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

2012-05-31 04:59:13

by Guoqing Jiang

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

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
>

2012-05-31 14:59:17

by Jon Hunter

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


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

2012-05-31 21:00:20

by Benoit Cousson

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

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