On 10/21/2013 04:05 AM, Mark Rutland wrote:
On Wed, Oct 02, 2013 at 07:44:32PM +0100, [email protected] wrote:
From: Dinh Nguyen <[email protected]>

Add device tree support to the DW watchdog timer.

Signed-off-by: Dinh Nguyen <[email protected]>
Acked-by: Jamie Iles <[email protected]>
Reviewed-by: Pavel Machek <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Jamie Iles <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Wim Van Sebroeck <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Stephen Warren <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
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/ :-)

"must be" or "should contain" ? I see both in various bindings.
Is there a preference ?

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

Is it the only interrupt?

+               clocks = <&per_base_clk>;

Similarly, is this the only clock?

The driver uses one clock, and it is mandatory.

Guenter

Is it necessary?

+               status = "okay";

This is unnecessary.

Thanks,
Mark.



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to