Re: [RESEND PATCHv2] watchdog: dw: Enable OF support for DW watchdog timer.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

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

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

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

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux