| Message ID | 527A28E8.4010204@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday 06 November 2013 06:32 AM, ivan.khoronzhuk wrote: > The Keystone arch is using clocks in DT and source clock for watchdog > has to be specified, so add this to binding. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> > --- Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
On 11/06/2013 03:32 AM, ivan.khoronzhuk wrote: > The Keystone arch is using clocks in DT and source clock for watchdog > has to be specified, so add this to binding. > > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> > --- > .../devicetree/bindings/watchdog/davinci-wdt.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt > index fddced9..4db4d0e 100644 > --- a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt > +++ b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt > @@ -7,6 +7,10 @@ Required properties: > > - reg : Should contain WDT registers location and length > > +- clocks: phandle reference to the controller clock. > + Required only for Keystone arch. > + See clock-bindings.txt > + Yet another form of formatting. Also, wonder if it makes sense to merge this with the patch adding keystone support. > Optional properties: > > - timeout-sec: Contains the watchdog timeout in seconds > @@ -21,4 +25,5 @@ wdt: wdt@2320000 { > compatible = "ti,davinci-wdt"; > reg = <0x02320000 0x80>; > timeout-sec = <30>; > + clocks = <&clkwdtimer0>; > }; >
On 11/17/2013 04:28 AM, Guenter Roeck wrote: > On 11/06/2013 03:32 AM, ivan.khoronzhuk wrote: >> The Keystone arch is using clocks in DT and source clock for watchdog >> has to be specified, so add this to binding. >> >> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> >> --- >> .../devicetree/bindings/watchdog/davinci-wdt.txt | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt >> b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt >> index fddced9..4db4d0e 100644 >> --- a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt >> +++ b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt >> @@ -7,6 +7,10 @@ Required properties: >> >> - reg : Should contain WDT registers location and length >> >> +- clocks: phandle reference to the controller clock. >> + Required only for Keystone arch. >> + See clock-bindings.txt >> + > > Yet another form of formatting. Also, wonder if it makes sense to merge > this with the patch adding keystone support. > Ok, I'll squash them. >> Optional properties: >> >> - timeout-sec: Contains the watchdog timeout in seconds >> @@ -21,4 +25,5 @@ wdt: wdt@2320000 { >> compatible = "ti,davinci-wdt"; >> reg = <0x02320000 0x80>; >> timeout-sec = <30>; >> + clocks = <&clkwdtimer0>; >> }; >> >
On Wednesday 06 November 2013, ivan.khoronzhuk wrote: > @@ -7,6 +7,10 @@ Required properties: > > - reg : Should contain WDT registers location and length > > +- clocks: phandle reference to the controller clock. > + Required only for Keystone arch. > + See clock-bindings.txt > + > Optional properties: > > - timeout-sec: Contains the watchdog timeout in seconds I think it should really be listed under "Optional properties" and the reference to Keystone removed. Note how the binding would need to change otherwise if another platform started to use the clock, which is a little silly. Arnd
On 11/23/2013 07:57 PM, Arnd Bergmann wrote: > On Wednesday 06 November 2013, ivan.khoronzhuk wrote: >> @@ -7,6 +7,10 @@ Required properties: >> >> - reg : Should contain WDT registers location and length >> >> +- clocks: phandle reference to the controller clock. >> + Required only for Keystone arch. >> + See clock-bindings.txt >> + >> Optional properties: >> >> - timeout-sec: Contains the watchdog timeout in seconds > > I think it should really be listed under "Optional properties" and the > reference to Keystone removed. Note how the binding would need > to change otherwise if another platform started to use the clock, which > is a little silly. > > Arnd > Ok, I'll move clocks property under "Optional properties" and describe it as following: Optional properties: - timeout-sec : Contains the watchdog timeout in seconds - clocks: phandle reference to the controller clock. Needed if platform uses clocks. See clock-bindings.txt FYI: The new patch series had been already presented, where the patches "watchdog: davinci: add "clocks" property" and "watchdog: davinci: reuse driver for keystone arch" were combined. http://www.spinics.net/lists/devicetree/msg12542.html
On Mon, Nov 25, 2013 at 10:59:45AM +0000, ivan.khoronzhuk wrote: > On 11/23/2013 07:57 PM, Arnd Bergmann wrote: > > On Wednesday 06 November 2013, ivan.khoronzhuk wrote: > >> @@ -7,6 +7,10 @@ Required properties: > >> > >> - reg : Should contain WDT registers location and length > >> > >> +- clocks: phandle reference to the controller clock. > >> + Required only for Keystone arch. > >> + See clock-bindings.txt > >> + > >> Optional properties: > >> > >> - timeout-sec: Contains the watchdog timeout in seconds > > > > I think it should really be listed under "Optional properties" and the > > reference to Keystone removed. Note how the binding would need > > to change otherwise if another platform started to use the clock, which > > is a little silly. > > > > Arnd > > > > Ok, I'll move clocks property under "Optional properties" and describe it > as following: > > Optional properties: > - timeout-sec : Contains the watchdog timeout in seconds > - clocks: phandle reference to the controller clock. > Needed if platform uses clocks. > See clock-bindings.txt Nit: clocks aren't just phandles, they have a clock-specifier too (which might be 0 cells). Otherwise this looks fine to me. Mark.
On 11/25/2013 02:15 PM, Mark Rutland wrote: > On Mon, Nov 25, 2013 at 10:59:45AM +0000, ivan.khoronzhuk wrote: >> On 11/23/2013 07:57 PM, Arnd Bergmann wrote: >>> On Wednesday 06 November 2013, ivan.khoronzhuk wrote: >>>> @@ -7,6 +7,10 @@ Required properties: >>>> >>>> - reg : Should contain WDT registers location and length >>>> >>>> +- clocks: phandle reference to the controller clock. >>>> + Required only for Keystone arch. >>>> + See clock-bindings.txt >>>> + >>>> Optional properties: >>>> >>>> - timeout-sec: Contains the watchdog timeout in seconds >>> >>> I think it should really be listed under "Optional properties" and the >>> reference to Keystone removed. Note how the binding would need >>> to change otherwise if another platform started to use the clock, which >>> is a little silly. >>> >>> Arnd >>> >> >> Ok, I'll move clocks property under "Optional properties" and describe it >> as following: >> >> Optional properties: >> - timeout-sec : Contains the watchdog timeout in seconds >> - clocks: phandle reference to the controller clock. >> Needed if platform uses clocks. >> See clock-bindings.txt > > Nit: clocks aren't just phandles, they have a clock-specifier too (which > might be 0 cells). > > Otherwise this looks fine to me. > > Mark. > ... I will replace it to: Optional properties: - timeout-sec : Contains the watchdog timeout in seconds - clocks: the clock feeding the watchdog timer. Needed if platform uses clocks. See clock-bindings.txt Is it OK?
diff --git a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt index fddced9..4db4d0e 100644 --- a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt +++ b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt @@ -7,6 +7,10 @@ Required properties: - reg : Should contain WDT registers location and length +- clocks: phandle reference to the controller clock. + Required only for Keystone arch. + See clock-bindings.txt + Optional properties: - timeout-sec: Contains the watchdog timeout in seconds @@ -21,4 +25,5 @@ wdt: wdt@2320000 { compatible = "ti,davinci-wdt"; reg = <0x02320000 0x80>; timeout-sec = <30>; + clocks = <&clkwdtimer0>; };
The Keystone arch is using clocks in DT and source clock for watchdog has to be specified, so add this to binding. Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@ti.com> --- .../devicetree/bindings/watchdog/davinci-wdt.txt | 5 +++++ 1 file changed, 5 insertions(+)