Re: [PATCH v3 3/3] dt-bindings: clock: lan966x: Add LAN966X Clock Controller

[Stephen Boyd]

Quoting Kavyasree Kotagiri (2021-09-09 00:39:47)
> This adds the DT bindings documentation for lan966x SoC
> generic clock controller.
> 
> Signed-off-by: Kavyasree Kotagiri <kavyasree.kotagiri@xxxxxxxxxxxxx>
> ---

This should come before the driver patch.

> v2 -> v3:
> - Fixed dt_binding_check errors.
> 
> v1 -> v2:
> - Updated example provided for clk controller DT node.
> 
>  .../bindings/clock/microchip,lan966x-gck.yaml | 64 +++++++++++++++++++
>  1 file changed, 64 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/microchip,lan966x-gck.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/microchip,lan966x-gck.yaml b/Documentation/devicetree/bindings/clock/microchip,lan966x-gck.yaml
> new file mode 100644
> index 000000000000..d353d42c3dc8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/microchip,lan966x-gck.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/microchip,lan966x-gck.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip LAN966X Generic Clock Controller
> +
> +maintainers:
> +  - Kavyasree Kotagiri <kavyasree.kotagiri@xxxxxxxxxxxxx>
> +
> +description: |
> +  The LAN966X Generic clock controller contains 3 PLLs - cpu_clk,
> +  ddr_clk and sys_clk. This clock controller generates and supplies
> +  clock to various peripherals within the SoC.
> +
> +properties:
> +  compatible:
> +    const: microchip,lan966x-gck
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 3

The order matters to the binding too so please indicate which clock goes
into which index, or use clock-names, or both.

> +
> +  '#clock-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    cpu_clk: cpu_clk {

node names should have dash in them, whereas labels should have
underscores.

> +        compatible = "fixed-clock";
> +        #clock-cells = <0>;
> +        clock-frequency = <600000000>;
> +    };
> +
> +    ddr_clk: ddr_clk {
> +        compatible = "fixed-clock";
> +        #clock-cells = <0>;
> +        clock-frequency = <300000000>;
> +    };
> +
> +    sys_clk: sys_clk {
> +        compatible = "fixed-clock";
> +        #clock-cells = <0>;
> +        clock-frequency = <162500000>;
> +    };

The fixed clks aren't necessary to put in the binding as they're just
used as phandles below. Please remove them from the example and leave
the node below intact with the phandles referencing "nothing".

> +
> +    clks: clock-controller@e00c00a8 {
> +        compatible = "microchip,lan966x-gck";
> +        #clock-cells = <1>;
> +        clocks = <&cpu_clk>, <&ddr_clk>, <&sys_clk>;
> +        reg = <0xe00c00a8 0x38>;
> +    };
> +...