On Wed, Dec 16, 2020 at 03:52:31PM +0100, Geert Uytterhoeven wrote:
> - Add reference to clock.yaml, and switch to unevaluatedProperties, to
> stop complaining about the presence of "assigned-clock-rates" and
> "assigned-clocks" in board DTS files,
> - Fix typo in "idt,voltage-microvolts" property name, to match example
> and driver code,
> - Add missing reference for "idt,voltage-microvolts",
> - Add missing "additionalProperties: false" for subnodes, to catch
> typos in properties,
> - There is no reason to wrap the (single) if condition in an allOf
> block,
True, but more future proof with it and unnecessary churn IMO.
> - Fix obsolete property names in example.
>
> Fixes: 45c940184b501fc6 ("dt-bindings: clk: versaclock5: convert to yaml")
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> Notes:
> 1. The use of "idt,voltage-microvolts" (with trailing S) is a bit
> unfortunate, as Documentation/devicetree/bindings/property-units.txt
> suggests to not have the trailing edge.
> Can we still fix the driver and bindings? While this entered
> uptstream in v5.9, there are no users in next-20201216.
>
> 2. Due to "clock-output-names" being part of
> dt-schema/schemas/clock/clock.yaml, the presence of this property
> does not trigger an error. Adding "clock-output-names: false"
> can fix that. But given this property is deprecated, except for
> very specific use cases, explicitly allowing it for those few use
> cases would be better.
> ---
> .../bindings/clock/idt,versaclock5.yaml | 53 ++++++++++---------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> index 2ac1131fd9222a86..14851e76f6342095 100644
> --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.yaml
> @@ -33,6 +33,9 @@ description: |
> maintainers:
> - Luca Ceresoli <luca@xxxxxxxxxxxxxxxx>
>
> +allOf:
> + - $ref: clock.yaml#
No, that's not right. clock.yaml is already applied unconditionally.
You need to define assigned-clocks, etc. here just like 'clocks' and
define how many entries. Or convince me they should be allowed on any
node.
> +
> properties:
> compatible:
> enum:
> @@ -73,40 +76,42 @@ patternProperties:
> $ref: /schemas/types.yaml#/definitions/uint32
> minimum: 0
> maximum: 6
> - idt,voltage-microvolt:
> + idt,voltage-microvolts:
Can we fix the driver? No in tree users...
> description: The output drive voltage.
> + $ref: /schemas/types.yaml#/definitions/uint32
> enum: [ 1800000, 2500000, 3300000 ]
> idt,slew-percent:
> description: The Slew rate control for CMOS single-ended.
> $ref: /schemas/types.yaml#/definitions/uint32
> enum: [ 80, 85, 90, 100 ]
>
> + additionalProperties: false
> +
> required:
> - compatible
> - reg
> - '#clock-cells'
>
> -allOf:
> - - if:
> - properties:
> - compatible:
> - enum:
> - - idt,5p49v5933
> - - idt,5p49v5935
> - then:
> - # Devices with builtin crystal + optional external input
> - properties:
> - clock-names:
> - const: clkin
> - clocks:
> - maxItems: 1
> - else:
> - # Devices without builtin crystal
> - required:
> - - clock-names
> - - clocks
> -
> -additionalProperties: false
> +if:
> + properties:
> + compatible:
> + enum:
> + - idt,5p49v5933
> + - idt,5p49v5935
> +then:
> + # Devices with builtin crystal + optional external input
> + properties:
> + clock-names:
> + const: clkin
> + clocks:
> + maxItems: 1
> +else:
> + # Devices without builtin crystal
> + required:
> + - clock-names
> + - clocks
> +
> +unevaluatedProperties: false
>
> examples:
> - |
> @@ -135,13 +140,13 @@ examples:
> clock-names = "xin";
>
> OUT1 {
> - idt,drive-mode = <VC5_CMOSD>;
> + idt,mode = <VC5_CMOSD>;
> idt,voltage-microvolts = <1800000>;
> idt,slew-percent = <80>;
> };
>
> OUT4 {
> - idt,drive-mode = <VC5_LVDS>;
> + idt,mode = <VC5_LVDS>;
> };
> };
> };
> --
> 2.25.1
>
