Re: [PATCH v1 1/2] dt-binding: clock: document NPCM7xx clock DT bindings

[Rob Herring]

On Mon, Feb 05, 2018 at 10:22:54AM +0200, Tomer Maimon wrote:
> Added device tree binding documentation for Nuvoton NPCM7xx clocks.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@xxxxxxxxx>
> ---
>  .../bindings/clock/nuvoton,npcm7xx-clk.txt         | 84 ++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/nuvoton,npcm7xx-clk.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/nuvoton,npcm7xx-clk.txt b/Documentation/devicetree/bindings/clock/nuvoton,npcm7xx-clk.txt
> new file mode 100644
> index 000000000000..1ba1945d3616
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/nuvoton,npcm7xx-clk.txt
> @@ -0,0 +1,84 @@
> +* Nuvoton NPCM7XX Clock Controller
> +
> +Nuvoton Poleg BMC NPCM7XX contain integrated clock

s/contain/contains/

And your line break is strange.

> +controller, which generates and supplies clock to all modules within the BMC.

s/clock/clocks/

> +
> +Required Properties:
> +
> +- compatible: should be one of following:
> +	- "nuvoton,npcm750-clk" : for clock controller of Nuvoton
> +	  Poleg BMC NPCM750
> +
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +
> +- #clock-cells: should be 1.
> +
> +All available clocks are defined as preprocessor macros in
> +dt-bindings/clock/nuvoton,npcm7xx-clock.h header and can beused in device tree

This file should be part of this patch.

> +sources.
> +
> +External clocks:
> +
> +There are several clocks that are generated outside the BMC. All clocks are of
> +a known fixed value that cannot be chagned. Therefor these values are hard coded

s/chagned/changed/

s/Therefor/Therefore/

> +inside the driver and registered on init.
> +
> +The clock modules contains 4 PLL, 20 dividers and 11 muxes. All these settings
> +are set before Linux boot and are not to be altered by the Linux. This driver is
> +used only to read the values clocks, not to set them.
> +
> +In addition to the clock driver, there are 3 external clocks suppling the

The binding describes h/w, not a driver.

> +network, which are of fixed values, set on on the device tree, but not used by
> + the clock module. Example can be found below.
   ^
extra space

All this description belongs at the top of this doc.

> +
> +Example: Clock controller node:
> +
> +	clk: clock-controller@f0801000 {
> +		compatible = "nuvoton,npcm750-clk";
> +		#clock-cells = <1>;
> +		clock-controller;
> +		reg = <0xf0801000 0x1000>;
> +		status = "okay";
> +	};
> +
> +Example: Required external clocks for network:
> +
> +		/* external clock signal rg1refck, supplied by the phy */
> +		clk_rg1refck: clk_rg1refck {

Use '-' rather than '_' in node names.

> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <125000000>;
> +			clock-output-names = "clk_rg1refck";
> +		};
> +
> +		/* external clock signal rg2refck, supplied by the phy */
> +		clk_rg2refck: clk_rg2refck {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <125000000>;
> +			clock-output-names = "clk_rg2refck";
> +		};
> +
> +		clk_xin: clk_xin {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			clock-frequency = <50000000>;
> +			clock-output-names = "clk_xin";
> +		};
> +
> +Example: UART controller node that consumes the clock generated by the clock
> +  controller (refer to the standard clock bindings for information about
> +  "clocks" and "clock-names" properties):
> +
> +	uart0: serial@e2900000 {
> +		compatible = "Nuvoton,s5pv210-uart";

s/Nuvoton/nuvoton/

> +		reg = <0xe2900000 0x400>;
> +		interrupt-parent = <&vic1>;
> +		interrupts = <10>;
> +		clock-names = "uart", "clk_uart_baud0",
> +				"clk_uart_baud1";
> +		clocks = <&clocks UART0>, <&clocks UART0>,
> +				<&clocks SCLK_UART0>;
> +	};
> -- 
> 2.14.1
>