Re: [PATCH v5 3/5] dt-bindings: clock: mediatek: add clock controllers of MT7988

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 12/12/2023 04:18, Daniel Golle wrote:
> Add various clock controllers found in the MT7988 SoC to existing
> bindings (if applicable) and add files for the new ethwarp, mcusys
> and xfi-pll clock controllers not previously present in any SoC.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---

...

> -      - const: syscon
> +    oneOf:
> +      - items:
> +          - enum:
> +              - mediatek,mt7622-sgmiisys
> +              - mediatek,mt7629-sgmiisys
> +              - mediatek,mt7981-sgmiisys_0
> +              - mediatek,mt7981-sgmiisys_1
> +              - mediatek,mt7986-sgmiisys_0
> +              - mediatek,mt7986-sgmiisys_1
> +          - const: syscon
> +      - items:
> +          - enum:
> +              - mediatek,mt7988-sgmiisys_0
> +              - mediatek,mt7988-sgmiisys_1

No underscores in compatibles. At least for new ones, because you cannot
touch the old.

> +          - const: simple-mfd
> +          - const: syscon
>  
>    reg:
>      maxItems: 1
> @@ -35,11 +42,47 @@ properties:
>      description: Invert polarity of the SGMII data lanes
>      type: boolean
>  
> +  pcs:
> +    type: object
> +    description: HSGMII PCS logic
> +    properties:
> +      compatible:
> +        const: mediatek,mt7988-sgmii
> +
> +      clocks:
> +        maxItems: 3
> +        minItems: 3

drop minItems

> +
> +      clock-names:
> +        items:
> +          - const: sgmii_sel
> +          - const: sgmii_tx
> +          - const: sgmii_rx
> +
> +    required:
> +      - compatible
> +      - clocks
> +      - clock-names
> +
> +    additionalProperties: false
> +
>  required:
>    - compatible
>    - reg
>    - '#clock-cells'
>  

add allOf: here

> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - mediatek,mt7988-sgmiisys_0
> +          - mediatek,mt7988-sgmiisys_1
> +
> +then:
> +  required:
> +    - pcs

else: also valid?

Maybe there is no point to have it in this binding?

Best regards,
Krzysztof