On Thu, Jan 19, 2017 at 09:59:50AM +0800, Baoyou Xie wrote: > This patch adds dt-binding documentation for zx2967 family > watchdog controller. > > Signed-off-by: Baoyou Xie <baoyou....@linaro.org>
It seems that the comments I put on v1 remains unresolved. > --- > .../bindings/watchdog/zte,zx2967-wdt.txt | 32 > ++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt > > diff --git a/Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt > b/Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt > new file mode 100644 > index 0000000..6e35ce7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/watchdog/zte,zx2967-wdt.txt > @@ -0,0 +1,32 @@ > +ZTE zx2967 Watchdog timer > + > +Required properties: > + > +- compatible : should be one of the following. > + * zte,zx296718-wdt > +- reg : Specifies base physical address and size of the registers. > +- clocks : Pairs of phandle and specifier referencing the controller's > clocks. > +- clock-names: "wdtclk" for the watchdog clock. > +- resets : Reference to the reset controller controlling the watchdog > + controller. > +- reset-names : Must include the following entries: > + * wdtrst I do not think clock-names and reset-names are really necessary, since there is only one clock and reset signal. > + > +Optional properties: > + > +- wdt-reset-sysctrl : should include following fields. We need a vendor prefix for vendor specific property. > + * phandle of aon-sysctrl. > + * configuare value that be wrote to aon-sysctrl. s/configuare/configure, s/wrote/written > + * bit mask, corresponding bits will be affected. I think we need some comments for bindings users to understand the different role between "resets" and "wdt-reset-sysctrl". Also, why is wdt-reset-sysctrl is optional? Does it still work well without this property. FYI. I have a similar bindings for TVENC below, which you may find useful. http://www.spinics.net/lists/devicetree/msg159668.html > + > +Example: > + > +wdt_ares: watchdog@1465000 { What does the suffix "ares" mean? I guess "wdt" is good enough as the label name. Shawn > + compatible = "zte,zx296718-wdt"; > + reg = <0x1465000 0x1000>; > + clocks = <&topcrm WDT_WCLK>; > + clock-names = "wdtclk"; > + resets = <&toprst 35>; > + reset-names = "wdtrst"; > + wdt-reset-sysctrl = <&aon_sysctrl 1 0x115>; > +}; > -- > 2.7.4 >
