On Tue, Oct 22, 2013 at 11:57:34AM +0100, Dinh Nguyen wrote: > Hi Mark, > > On 10/22/13 5:39 AM, Mark Rutland wrote: > > On Tue, Oct 22, 2013 at 01:41:20AM +0100, Guenter Roeck wrote: > >> On 10/21/2013 04:05 AM, Mark Rutland wrote: > >>> On Wed, Oct 02, 2013 at 07:44:32PM +0100, dinguyen@xxxxxxxxxx wrote: > >>>> From: Dinh Nguyen <dinguyen@xxxxxxxxxx> > >>>> > >>>> Add device tree support to the DW watchdog timer. > >>>> > >>>> Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxx> > >>>> Acked-by: Jamie Iles <jamie@xxxxxxxxxxxxx> > >>>> Reviewed-by: Pavel Machek <pavel@xxxxxxx> > >>>> Cc: Guenter Roeck <linux@xxxxxxxxxxxx> > >>>> Cc: Jamie Iles <jamie@xxxxxxxxxxxxx> > >>>> Cc: Viresh Kumar <viresh.linux@xxxxxxxxx> > >>>> Cc: Wim Van Sebroeck <wim@xxxxxxxxx> > >>>> Cc: Pavel Machek <pavel@xxxxxxx> > >>>> Cc: Rob Herring <rob.herring@xxxxxxxxxxx> > >>>> Cc: Pawel Moll <pawel.moll@xxxxxxx> > >>>> Cc: Mark Rutland <mark.rutland@xxxxxxx> > >>>> Cc: Stephen Warren <swarren@xxxxxxxxxxxxx> > >>>> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx> > >>>> Cc: devicetree@xxxxxxxxxxxxxxx > >>>> Cc: linux-watchdog@xxxxxxxxxxxxxxx > >>>> --- > >>>> v2: > >>>> - Use of_match_ptr() for of_match_table > >>>> --- > >>>> .../devicetree/bindings/watchdog/dw_wdt.txt | 16 ++++++++++++++++ > >>>> drivers/watchdog/dw_wdt.c | 8 ++++++++ > >>>> 2 files changed, 24 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/watchdog/dw_wdt.txt > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/watchdog/dw_wdt.txt b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt > >>>> new file mode 100644 > >>>> index 0000000..29e150b > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/watchdog/dw_wdt.txt > >>>> @@ -0,0 +1,16 @@ > >>>> +Synopsys Designware Watchdog Timer > >>>> + > >>>> +Required Properties: > >>>> + > >>>> +- Compatiblity : "snps,dw-wdt" > >>> This should presumably be: > >>> > >>> - compatbile: should contain "snps,dw-wdt" > >>> > >> Hi Mark, > >> > >> s/compatbile/compatible/ :-) > > Indeed :) > Changed for v3. > > > >> "must be" or "should contain" ? I see both in various bindings. > >> Is there a preference ? > > I would prefer "should contain" -- it doesn't preclude future compatible > > variants. > Oops..v3 has "should be"...will change to "should contain" in v4. Cheers. > > > >>>> +- reg : Base address of the watchdog timer register. > >>> And the size... > >>> > >>>> + > >>>> +Example: > >>>> + > >>>> + watchdog0: wd@ffd02000 { > >>>> + compatible = "snps,dw-wdt"; > >>>> + reg = <0xffd02000 0x1000>; > >>>> + interrupts = <0 171 4>; > >>> This wasn't mentioned. > >>> > >>> Is it necessary? > >>> > >> From looking into the code ... > >> > >> The driver doesn't use interrupts, so I guess the answer is no. Cut-and-paste error, maybe ? > > Does the device have any interrupts, even if they're unused? > Yes, the device does have an interrupt. Should I add it as an optional > property? Yes please, given it's easy to add. We might want to use it in future so it would be nice to have now. > > > >>> Is it the only interrupt? > >>> > >>>> + clocks = <&per_base_clk>; > >>> Similarly, is this the only clock? > >>> > >> The driver uses one clock, and it is mandatory. > > Is this the only clock into the unit? > > > > Does it have a name? > Yes, this is is the only clock into the unit. No, it does not have name. Ok. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html