[PATCH 1/1] dt-bindings: clock: ti,cdce925: Convert to DT schema

[PATCH 1/1] dt-bindings: clock: ti,cdce925: Convert to DT schema

[Alexander Stein]

Convert the TI CDCE925 clock binding to DT schema format.
Including a small fix: Add the missing 'ti' prefix in the example
compatible.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
---
I have to admit I only have one specific addon platform for this
hardware, which is actually a CECD813 tbh.

 .../devicetree/bindings/clock/ti,cdce925.txt  |  53 ---------
 .../devicetree/bindings/clock/ti,cdce925.yaml | 104 ++++++++++++++++++
 2 files changed, 104 insertions(+), 53 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.txt
 create mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.yaml

diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.txt b/Documentation/devicetree/bindings/clock/ti,cdce925.txt
deleted file mode 100644
index df42ab72718f..000000000000
--- a/Documentation/devicetree/bindings/clock/ti,cdce925.txt
+++ /dev/null
@@ -1,53 +0,0 @@
-Binding for TI CDCE913/925/937/949 programmable I2C clock synthesizers.
-
-Reference
-This binding uses the common clock binding[1].
-
-[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
-[2] https://www.ti.com/product/cdce913
-[3] https://www.ti.com/product/cdce925
-[4] https://www.ti.com/product/cdce937
-[5] https://www.ti.com/product/cdce949
-
-The driver provides clock sources for each output Y1 through Y5.
-
-Required properties:
- - compatible: Shall be one of the following:
-	- "ti,cdce913": 1-PLL, 3 Outputs
-	- "ti,cdce925": 2-PLL, 5 Outputs
-	- "ti,cdce937": 3-PLL, 7 Outputs
-	- "ti,cdce949": 4-PLL, 9 Outputs
- - reg: I2C device address.
- - clocks: Points to a fixed parent clock that provides the input frequency.
- - #clock-cells: From common clock bindings: Shall be 1.
-
-Optional properties:
- - xtal-load-pf: Crystal load-capacitor value to fine-tune performance on a
-                 board, or to compensate for external influences.
-- vdd-supply: A regulator node for Vdd
-- vddout-supply: A regulator node for Vddout
-
-For all PLL1, PLL2, ... an optional child node can be used to specify spread
-spectrum clocking parameters for a board.
-  - spread-spectrum: SSC mode as defined in the data sheet.
-  - spread-spectrum-center: Use "centered" mode instead of "max" mode. When
-    present, the clock runs at the requested frequency on average. Otherwise
-    the requested frequency is the maximum value of the SCC range.
-
-
-Example:
-
-	clockgen: cdce925pw@64 {
-		compatible = "cdce925";
-		reg = <0x64>;
-		clocks = <&xtal_27Mhz>;
-		#clock-cells = <1>;
-		xtal-load-pf = <5>;
-		vdd-supply = <&1v8-reg>;
-		vddout-supply = <&3v3-reg>;
-		/* PLL options to get SSC 1% centered */
-		PLL2 {
-			spread-spectrum = <4>;
-			spread-spectrum-center;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
new file mode 100644
index 000000000000..1e68ee68e458
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node bindings
+
+maintainers:
+  - Mike Looijmans <mike.looijmans@topic.nl>
+
+description: |
+  Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI Reduction
+
+  - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913
+  - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925
+  - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937
+  - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949
+
+properties:
+  $nodename:
+    pattern: "^clock-controller$"
+
+  compatible:
+    enum:
+      - ti,cdce913
+      - ti,cdce925
+      - ti,cdce937
+      - ti,cdce949
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: fixed parent clock
+
+  "#clock-cells":
+    const: 1
+
+  vdd-supply:
+    description: Regulator that provides 1.8V Vdd power supply
+
+  vddout-supply:
+    description: |
+      Regulator that provides Vddout power supply.
+      non-L variant: 2.5V or 3.3V for
+      L variant: 1.8V for
+
+  xtal-load-pf:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Crystal load-capacitor value to fine-tune performance on a
+      board, or to compensate for external influences.
+
+patternProperties:
+  "^PLL[1-4]$":
+    type: object
+    description: |
+      optional child node can be used to specify spread
+      spectrum clocking parameters for a board
+
+    properties:
+      spread-spectrum:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description: SSC mode as defined in the data sheet
+
+      spread-spectrum-center:
+        type: boolean
+        description: |
+          Use "centered" mode instead of "max" mode. When
+          present, the clock runs at the requested frequency on average.
+          Otherwise the requested frequency is the maximum value of the
+          SCC range.
+
+required:
+  - compatible
+  - ret
+  - clocks
+  - "#clock-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        cdce925: clock-controller@64 {
+            compatible = "ti,cdce925";
+            reg = <0x64>;
+            clocks = <&xtal_27Mhz>;
+            #clock-cells = <1>;
+            xtal-load-pf = <5>;
+            vdd-supply = <&reg_1v8>;
+            vddout-supply = <&reg_3v3>;
+            /* PLL options to get SSC 1% centered */
+            PLL2 {
+                spread-spectrum = <4>;
+                spread-spectrum-center;
+            };
+        };
+    };
-- 
2.25.1

Re: [PATCH 1/1] dt-bindings: clock: ti,cdce925: Convert to DT schema

[Rob Herring]

On Tue, 18 Oct 2022 09:21:06 +0200, Alexander Stein wrote:
> Convert the TI CDCE925 clock binding to DT schema format.
> Including a small fix: Add the missing 'ti' prefix in the example
> compatible.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> I have to admit I only have one specific addon platform for this
> hardware, which is actually a CECD813 tbh.
> 
>  .../devicetree/bindings/clock/ti,cdce925.txt  |  53 ---------
>  .../devicetree/bindings/clock/ti,cdce925.yaml | 104 ++++++++++++++++++
>  2 files changed, 104 insertions(+), 53 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/ti,cdce925.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.example.dtb: clock-controller@64: $nodename:0: 'clock-controller@64' does not match '^clock-controller$'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.example.dtb: clock-controller@64: 'ret' is a required property
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/ti,cdce925.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 1/1] dt-bindings: clock: ti,cdce925: Convert to DT schema

[Krzysztof Kozlowski]

On 18/10/2022 03:21, Alexander Stein wrote:
> Convert the TI CDCE925 clock binding to DT schema format.
> Including a small fix: Add the missing 'ti' prefix in the example
> compatible.
> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>

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

> diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
> new file mode 100644
> index 000000000000..1e68ee68e458
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node bindings

Drop "node bindings"

> +
> +maintainers:
> +  - Mike Looijmans <mike.looijmans@topic.nl>
> +
> +description: |
> +  Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI Reduction
> +
> +  - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913
> +  - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925
> +  - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937
> +  - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949
> +
> +properties:
> +  $nodename:
> +    pattern: "^clock-controller$"

Drop this requirement. It is in general expected, but there is no need
for each binding to specify it.

Other problem is that you did not actually test these bindings before
sending...

> +
> +  compatible:
> +    enum:
> +      - ti,cdce913
> +      - ti,cdce925
> +      - ti,cdce937
> +      - ti,cdce949
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: fixed parent clock
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  vdd-supply:
> +    description: Regulator that provides 1.8V Vdd power supply
> +
> +  vddout-supply:
> +    description: |
> +      Regulator that provides Vddout power supply.
> +      non-L variant: 2.5V or 3.3V for
> +      L variant: 1.8V for
> +
> +  xtal-load-pf:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Crystal load-capacitor value to fine-tune performance on a
> +      board, or to compensate for external influences.
> +
> +patternProperties:
> +  "^PLL[1-4]$":
> +    type: object
> +    description: |
> +      optional child node can be used to specify spread
> +      spectrum clocking parameters for a board
> +

    additionalProperties: false

> +    properties:
> +      spread-spectrum:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description: SSC mode as defined in the data sheet
> +
> +      spread-spectrum-center:
> +        type: boolean
> +        description: |
> +          Use "centered" mode instead of "max" mode. When
> +          present, the clock runs at the requested frequency on average.
> +          Otherwise the requested frequency is the maximum value of the
> +          SCC range.
> +

Best regards,
Krzysztof

Re: Re: [PATCH 1/1] dt-bindings: clock: ti,cdce925: Convert to DT schema

[Alexander Stein]

Hi Krzysztof,

thanks for your review.

Am Dienstag, 18. Oktober 2022, 15:51:35 CEST schrieb Krzysztof Kozlowski:
> On 18/10/2022 03:21, Alexander Stein wrote:
> > Convert the TI CDCE925 clock binding to DT schema format.
> > Including a small fix: Add the missing 'ti' prefix in the example
> > compatible.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> > diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
> > b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml new file mode
> > 100644
> > index 000000000000..1e68ee68e458
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
> > @@ -0,0 +1,104 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node
> > bindings
> Drop "node bindings"

Thanks, will do so.

> > +
> > +maintainers:
> > +  - Mike Looijmans <mike.looijmans@topic.nl>
> > +
> > +description: |
> > +  Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI
> > Reduction +
> > +  - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913
> > +  - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925
> > +  - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937
> > +  - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^clock-controller$"
> 
> Drop this requirement. It is in general expected, but there is no need
> for each binding to specify it.

Should this be put in a common binding then?

> Other problem is that you did not actually test these bindings before
> sending...
> 
> > +
> > +  compatible:
> > +    enum:
> > +      - ti,cdce913
> > +      - ti,cdce925
> > +      - ti,cdce937
> > +      - ti,cdce949
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: fixed parent clock
> > +
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  vdd-supply:
> > +    description: Regulator that provides 1.8V Vdd power supply
> > +
> > +  vddout-supply:
> > +    description: |
> > +      Regulator that provides Vddout power supply.
> > +      non-L variant: 2.5V or 3.3V for
> > +      L variant: 1.8V for
> > +
> > +  xtal-load-pf:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      Crystal load-capacitor value to fine-tune performance on a
> > +      board, or to compensate for external influences.
> > +
> > +patternProperties:
> > +  "^PLL[1-4]$":
> > +    type: object
> > +    description: |
> > +      optional child node can be used to specify spread
> > +      spectrum clocking parameters for a board
> > +
> 
>     additionalProperties: false

Will do.

Thanks and best regards,
Alexander

> > +    properties:
> > +      spread-spectrum:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        description: SSC mode as defined in the data sheet
> > +
> > +      spread-spectrum-center:
> > +        type: boolean
> > +        description: |
> > +          Use "centered" mode instead of "max" mode. When
> > +          present, the clock runs at the requested frequency on average.
> > +          Otherwise the requested frequency is the maximum value of the
> > +          SCC range.
> > +
> 
> Best regards,
> Krzysztof

Re: [PATCH 1/1] dt-bindings: clock: ti,cdce925: Convert to DT schema

[Krzysztof Kozlowski]

On 19/10/2022 01:49, Alexander Stein wrote:
> Hi Krzysztof,
> 
> thanks for your review.
> 
> Am Dienstag, 18. Oktober 2022, 15:51:35 CEST schrieb Krzysztof Kozlowski:
>> On 18/10/2022 03:21, Alexander Stein wrote:
>>> Convert the TI CDCE925 clock binding to DT schema format.
>>> Including a small fix: Add the missing 'ti' prefix in the example
>>> compatible.
>>>
>>> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
>>
>> Thank you for your patch. There is something to discuss/improve.
>>
>>> diff --git a/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
>>> b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml new file mode
>>> 100644
>>> index 000000000000..1e68ee68e458
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/ti,cdce925.yaml
>>> @@ -0,0 +1,104 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/ti,cdce925.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: TI CDCE913/925/937/949 programmable I2C clock synthesizers node
>>> bindings
>> Drop "node bindings"
> 
> Thanks, will do so.
> 
>>> +
>>> +maintainers:
>>> +  - Mike Looijmans <mike.looijmans@topic.nl>
>>> +
>>> +description: |
>>> +  Flexible Low Power LVCMOS Clock Generator with SSC Support for EMI
>>> Reduction +
>>> +  - CDCE(L)913: 1-PLL, 3 Outputs https://www.ti.com/product/cdce913
>>> +  - CDCE(L)925: 2-PLL, 5 Outputs https://www.ti.com/product/cdce925
>>> +  - CDCE(L)937: 3-PLL, 7 Outputs https://www.ti.com/product/cdce937
>>> +  - CDCE(L)949: 4-PLL, 9 Outputs https://www.ti.com/product/cdce949
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    pattern: "^clock-controller$"
>>
>> Drop this requirement. It is in general expected, but there is no need
>> for each binding to specify it.
> 
> Should this be put in a common binding then?

Could be but we do not have dedicated binding for clock controllers, so
this would be a common binding only for name. :)


Best regards,
Krzysztof