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

From: Rob Herring
Date: Thu Feb 08 2018 - 21:22:59 EST


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
>