Re: [PATCH 03/11] dt-bindings: arm: sp810: convert to DT schema

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Andre Przywara <andre.przywara@arm.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Liviu Dudau <liviu.dudau@arm.com>,
	Robin Murphy <robin.murphy@arm.com>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 03/11] dt-bindings: arm: sp810: convert to DT schema
Date: Thu, 28 Apr 2022 09:01:35 +0200	[thread overview]
Message-ID: <c126ca43-e9f9-0fcb-fa7e-2168625ae25f@linaro.org> (raw)
In-Reply-To: <20220427112528.4097815-4-andre.przywara@arm.com>

On 27/04/2022 13:25, Andre Przywara wrote:

Thank you for your patch. There is something to discuss/improve.

> +  reg:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: refclk
> +      - const: timclk
> +      - const: apb_pclk
> +
> +  clocks:
> +    items:
> +      - description: reference clock
> +      - description: timer clock
> +      - description: APB register access clock
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  clock-output-names:
> +    items:
> +      - const: timerclken0
> +      - const: timerclken1
> +      - const: timerclken2
> +      - const: timerclken3
> +
> +  assigned-clocks:
> +    minItems: 4
> +
> +  assigned-clock-parents:
> +    minItems: 4
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"

clocks, clock-names, clock-output-names, assigned-clocks,
assigned-clock-parents. They were all required and relaxing was not
described/justified in commit msg.

> +
> +examples:
> +  - |
> +    sysctl@20000 {
> +            compatible = "arm,sp810", "arm,primecell";

Indentation is messed up here. 4 spaces please.

> +            reg = <0x020000 0x1000>;
> +            clocks = <&v2m_refclk32khz>, <&v2m_refclk1mhz>, <&smbclk>;
> +            clock-names = "refclk", "timclk", "apb_pclk";
> +            #clock-cells = <1>;
> +            clock-output-names = "timerclken0", "timerclken1",
> +                                 "timerclken2", "timerclken3";
> +            assigned-clocks = <&v2m_sysctl 0>, <&v2m_sysctl 1>,
> +                              <&v2m_sysctl 3>, <&v2m_sysctl 3>;
> +            assigned-clock-parents = <&v2m_refclk1mhz>, <&v2m_refclk1mhz>,
> +                                     <&v2m_refclk1mhz>, <&v2m_refclk1mhz>;
> +    };

Best regards,
Krzysztof