* [PATCH 0/3] omap3/omap4: add device tree support for wdt
@ 2012-05-25 10:42 jgq516 at gmail.com
2012-05-25 10:42 ` [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 jgq516 at gmail.com
` (3 more replies)
0 siblings, 4 replies; 21+ messages in thread
From: jgq516 at gmail.com @ 2012-05-25 10:42 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 2012-05-25 10:42 [PATCH 0/3] omap3/omap4: add device tree support for wdt jgq516 at gmail.com @ 2012-05-25 10:42 ` jgq516 at gmail.com 2012-05-29 17:52 ` Jon Hunter 2012-05-25 10:42 ` [PATCH 2/3] OMAP: avoid build wdt platform device if with dt support jgq516 at gmail.com ` (2 subsequent siblings) 3 siblings, 1 reply; 21+ messages in thread From: jgq516 at gmail.com @ 2012-05-25 10:42 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 at 4a314000{ + compatible = "ti,omap4-wdt"; + ti,hwmods = "wd_timer2"; + }; }; }; -- 1.7.3 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 2012-05-25 10:42 ` [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 jgq516 at gmail.com @ 2012-05-29 17:52 ` Jon Hunter 2012-05-30 3:19 ` Xiao Jiang 0 siblings, 1 reply; 21+ messages in thread From: Jon Hunter @ 2012-05-29 17:52 UTC (permalink / raw) To: linux-arm-kernel On 05/25/2012 05:42 AM, jgq516 at 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 at 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 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 2012-05-29 17:52 ` Jon Hunter @ 2012-05-30 3:19 ` Xiao Jiang 2012-05-30 14:42 ` Jon Hunter 0 siblings, 1 reply; 21+ messages in thread From: Xiao Jiang @ 2012-05-30 3:19 UTC (permalink / raw) To: linux-arm-kernel Jon Hunter wrote: > On 05/25/2012 05:42 AM, jgq516 at 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 at 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 at 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 at 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 at 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 > ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 2012-05-30 3:19 ` Xiao Jiang @ 2012-05-30 14:42 ` Jon Hunter 2012-05-31 5:51 ` Xiao Jiang 0 siblings, 1 reply; 21+ messages in thread From: Jon Hunter @ 2012-05-30 14:42 UTC (permalink / raw) To: linux-arm-kernel On 05/29/2012 10:19 PM, Xiao Jiang wrote: > Jon Hunter wrote: >> On 05/25/2012 05:42 AM, jgq516 at 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 at 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 at 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 at 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 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 2012-05-30 14:42 ` Jon Hunter @ 2012-05-31 5:51 ` Xiao Jiang 2012-05-31 14:55 ` Jon Hunter 0 siblings, 1 reply; 21+ messages in thread From: Xiao Jiang @ 2012-05-31 5:51 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 at localhost:/root> dmesg|grep Machine [ 0.000000] Machine: Generic OMAP4 (Flattened Device Tree), model: TI OMAP4 SDP board root at 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 > ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 2012-05-31 5:51 ` Xiao Jiang @ 2012-05-31 14:55 ` Jon Hunter 2012-05-31 20:59 ` Cousson, Benoit 0 siblings, 1 reply; 21+ messages in thread From: Jon Hunter @ 2012-05-31 14:55 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 at localhost:/root> dmesg|grep Machine > [ 0.000000] Machine: Generic OMAP4 (Flattened Device Tree), model: TI > OMAP4 SDP board > root at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 2012-05-31 14:55 ` Jon Hunter @ 2012-05-31 20:59 ` Cousson, Benoit 0 siblings, 0 replies; 21+ messages in thread From: Cousson, Benoit @ 2012-05-31 20:59 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 at localhost:/root> dmesg|grep Machine >> [ 0.000000] Machine: Generic OMAP4 (Flattened Device Tree), model: TI >> OMAP4 SDP board >> root at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/3] OMAP: avoid build wdt platform device if with dt support 2012-05-25 10:42 [PATCH 0/3] omap3/omap4: add device tree support for wdt jgq516 at gmail.com 2012-05-25 10:42 ` [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 jgq516 at gmail.com @ 2012-05-25 10:42 ` jgq516 at gmail.com 2012-05-29 17:53 ` Jon Hunter 2012-05-25 10:42 ` [PATCH 3/3] watchdog: omap_wdt: add device tree support jgq516 at gmail.com 2012-05-29 17:47 ` [PATCH 0/3] omap3/omap4: add device tree support for wdt Jon Hunter 3 siblings, 1 reply; 21+ messages in thread From: jgq516 at gmail.com @ 2012-05-25 10:42 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/3] OMAP: avoid build wdt platform device if with dt support 2012-05-25 10:42 ` [PATCH 2/3] OMAP: avoid build wdt platform device if with dt support jgq516 at gmail.com @ 2012-05-29 17:53 ` Jon Hunter 0 siblings, 0 replies; 21+ messages in thread From: Jon Hunter @ 2012-05-29 17:53 UTC (permalink / raw) To: linux-arm-kernel On 05/25/2012 05:42 AM, jgq516 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-25 10:42 [PATCH 0/3] omap3/omap4: add device tree support for wdt jgq516 at gmail.com 2012-05-25 10:42 ` [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 jgq516 at gmail.com 2012-05-25 10:42 ` [PATCH 2/3] OMAP: avoid build wdt platform device if with dt support jgq516 at gmail.com @ 2012-05-25 10:42 ` jgq516 at gmail.com 2012-05-29 18:06 ` Jon Hunter 2012-05-29 17:47 ` [PATCH 0/3] omap3/omap4: add device tree support for wdt Jon Hunter 3 siblings, 1 reply; 21+ messages in thread From: jgq516 at gmail.com @ 2012-05-25 10:42 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-25 10:42 ` [PATCH 3/3] watchdog: omap_wdt: add device tree support jgq516 at gmail.com @ 2012-05-29 18:06 ` Jon Hunter 2012-05-30 3:18 ` Xiao Jiang 0 siblings, 1 reply; 21+ messages in thread From: Jon Hunter @ 2012-05-29 18:06 UTC (permalink / raw) To: linux-arm-kernel On 05/25/2012 05:42 AM, jgq516 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-29 18:06 ` Jon Hunter @ 2012-05-30 3:18 ` Xiao Jiang 2012-05-30 7:54 ` Cousson, Benoit 0 siblings, 1 reply; 21+ messages in thread From: Xiao Jiang @ 2012-05-30 3:18 UTC (permalink / raw) To: linux-arm-kernel Jon Hunter wrote: > On 05/25/2012 05:42 AM, jgq516 at 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 > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-30 3:18 ` Xiao Jiang @ 2012-05-30 7:54 ` Cousson, Benoit 2012-05-30 10:14 ` Xiao Jiang 2012-05-30 15:03 ` Jon Hunter 0 siblings, 2 replies; 21+ messages in thread From: Cousson, Benoit @ 2012-05-30 7:54 UTC (permalink / raw) To: linux-arm-kernel On 5/30/2012 5:18 AM, Xiao Jiang wrote: > Jon Hunter wrote: >> On 05/25/2012 05:42 AM, jgq516 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-30 7:54 ` Cousson, Benoit @ 2012-05-30 10:14 ` Xiao Jiang 2012-05-30 10:31 ` Xiao Jiang 2012-05-30 15:03 ` Jon Hunter 1 sibling, 1 reply; 21+ messages in thread From: Xiao Jiang @ 2012-05-30 10:14 UTC (permalink / raw) To: linux-arm-kernel Cousson, Benoit wrote: > On 5/30/2012 5:18 AM, Xiao Jiang wrote: >> Jon Hunter wrote: >>> On 05/25/2012 05:42 AM, jgq516 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-30 10:14 ` Xiao Jiang @ 2012-05-30 10:31 ` Xiao Jiang 0 siblings, 0 replies; 21+ messages in thread From: Xiao Jiang @ 2012-05-30 10:31 UTC (permalink / raw) To: linux-arm-kernel >> 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-30 7:54 ` Cousson, Benoit 2012-05-30 10:14 ` Xiao Jiang @ 2012-05-30 15:03 ` Jon Hunter 2012-05-30 15:30 ` Cousson, Benoit 1 sibling, 1 reply; 21+ messages in thread From: Jon Hunter @ 2012-05-30 15:03 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-30 15:03 ` Jon Hunter @ 2012-05-30 15:30 ` Cousson, Benoit 2012-05-30 16:12 ` Jon Hunter 0 siblings, 1 reply; 21+ messages in thread From: Cousson, Benoit @ 2012-05-30 15:30 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] watchdog: omap_wdt: add device tree support 2012-05-30 15:30 ` Cousson, Benoit @ 2012-05-30 16:12 ` Jon Hunter 0 siblings, 0 replies; 21+ messages in thread From: Jon Hunter @ 2012-05-30 16:12 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 0/3] omap3/omap4: add device tree support for wdt 2012-05-25 10:42 [PATCH 0/3] omap3/omap4: add device tree support for wdt jgq516 at gmail.com ` (2 preceding siblings ...) 2012-05-25 10:42 ` [PATCH 3/3] watchdog: omap_wdt: add device tree support jgq516 at gmail.com @ 2012-05-29 17:47 ` Jon Hunter 2012-05-30 10:14 ` Xiao Jiang 3 siblings, 1 reply; 21+ messages in thread From: Jon Hunter @ 2012-05-29 17:47 UTC (permalink / raw) To: linux-arm-kernel Hi Xiao Jiang, On 05/25/2012 05:42 AM, jgq516 at 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(-) > ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 0/3] omap3/omap4: add device tree support for wdt 2012-05-29 17:47 ` [PATCH 0/3] omap3/omap4: add device tree support for wdt Jon Hunter @ 2012-05-30 10:14 ` Xiao Jiang 0 siblings, 0 replies; 21+ messages in thread From: Xiao Jiang @ 2012-05-30 10:14 UTC (permalink / raw) To: linux-arm-kernel Jon Hunter wrote: > Hi Xiao Jiang, > > On 05/25/2012 05:42 AM, jgq516 at 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2012-05-31 20:59 UTC | newest] Thread overview: 21+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-25 10:42 [PATCH 0/3] omap3/omap4: add device tree support for wdt jgq516 at gmail.com 2012-05-25 10:42 ` [PATCH 1/3] arm/dts: add wdt node for omap3 and omap4 jgq516 at gmail.com 2012-05-29 17:52 ` Jon Hunter 2012-05-30 3:19 ` Xiao Jiang 2012-05-30 14:42 ` Jon Hunter 2012-05-31 5:51 ` Xiao Jiang 2012-05-31 14:55 ` Jon Hunter 2012-05-31 20:59 ` Cousson, Benoit 2012-05-25 10:42 ` [PATCH 2/3] OMAP: avoid build wdt platform device if with dt support jgq516 at gmail.com 2012-05-29 17:53 ` Jon Hunter 2012-05-25 10:42 ` [PATCH 3/3] watchdog: omap_wdt: add device tree support jgq516 at gmail.com 2012-05-29 18:06 ` Jon Hunter 2012-05-30 3:18 ` Xiao Jiang 2012-05-30 7:54 ` Cousson, Benoit 2012-05-30 10:14 ` Xiao Jiang 2012-05-30 10:31 ` Xiao Jiang 2012-05-30 15:03 ` Jon Hunter 2012-05-30 15:30 ` Cousson, Benoit 2012-05-30 16:12 ` Jon Hunter 2012-05-29 17:47 ` [PATCH 0/3] omap3/omap4: add device tree support for wdt Jon Hunter 2012-05-30 10:14 ` Xiao Jiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).