Re: [PATCH v3 1/2] dt-binding: clock: cs2600: Add support for the CS2600

[Krzysztof Kozlowski <krzk@xxxxxxxxxx>]

On Fri, Dec 27, 2024 at 04:49:14PM -0600, Paul Handrigan wrote:
> Add device tree support for the Cirrus Logic CS2600 clock
> device.
> 
> Signed-off-by: Paul Handrigan <paulha@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  .../bindings/clock/cirrus,cs2600.yaml         | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml b/Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml
> new file mode 100644
> index 000000000000..0be613c78a9c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/cirrus,cs2600.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/cirrus,cs2600.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic Fractional-N Clock Synthesizer & Clock Multiplier
> +
> +maintainers:
> +  - Paul Handrigan <paulha@xxxxxxxxxxxxxxxxxxxxx>
> +  - patches@xxxxxxxxxxxxxxxxxxxxx>
> +
> +description:
> +  The CS2600 is a system-clocking device that enables frequency synthesis and
> +  clock generation from a stable timing reference clock. The device can
> +  generate low-jitter clocks from a noisy clock reference at frequencies
> +  as low as 50 Hz.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - cirrus,cs2600
> +
> +  clocks:
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      enum:
> +        - xti
> +        - clk_in
> +    minItems: 1
> +    maxItems: 2

This does not match clocks. Nothing here, neither commit msg nor
description, explains why this is so flexible.


> +
> +  '#clock-cells':
> +    const: 1
> +
> +  reg:
> +    maxItems: 1

Keep the same order of properties as in required list.

> +
> +  vdd-supply:
> +    description: Power Supply
> +
> +  clock-output-names:
> +    maxItems: 3
> +    description: Names for CLK_OUT, BCLK_OUT and FSYNC_OUT clocks.
> +
> +  cirrus,aux-output-source:
> +    description:
> +      Specifies the function of the auxiliary output pin with "PHASE_UNLOCK"
> +      to indicate the input and output clocks are not in phase, "FREQ_UNLOCK"
> +      to indicate the PLL is unlocked, and NO_CLKIN to indicate the clock on
> +      the clk_in pin is not present.

enum and lower-case

> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +  cirrus,clock-mode:
> +    description:
> +      Sets the device into smart mode whith "SMART_MODE" and sets the device
> +      into smart mode that only outputs a clock when clk_in starts with
> +      "SMART_CLKIN_ONLY_MODE".

enum and lower-case

> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - '#clock-cells'
> +
> +unevaluatedProperties: false

This is supposed to be additionalProperties. See example-schema.

> +
> +examples:
> +  - |
> +

Drop blank line

> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;

Best regards,
Krzysztof