Re: [PATCH v2 4/4] dt-bindings: clk: versaclock5: convert to yaml

[Rob Herring <robh@kernel.org>]

On Tue, Jul 14, 2020 at 3:15 AM Luca Ceresoli <luca@lucaceresoli.net> wrote:
>
> Hi Rob,
>
> thanks for you review!
>
> On 14/07/20 05:11, Rob Herring wrote:
> > On Wed, Jul 08, 2020 at 09:40:35AM +0200, Luca Ceresoli wrote:
> >> Convert to yaml the VersaClock bindings document. The mapping between
> >> clock specifier and physical pins cannot be described formally in yaml
> >> schema, then keep it verbatim in the description field.
> >>
> >> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>
> [...]
>
> >> +  reg:
> >> +    maxItems: 1
> >> +    description: I2C device address, shall be 0x68 or 0x6a.
> >
> > Can be a schema:
> >
> > enum: [ 0x68, 0x6a ]
>
> Nice, will fix.
>
> >> +
> >> +  '#clock-cells':
> >> +    const: 1
> >> +
> >> +patternProperties:
> >> +  "^OUT[1-4]$":
> >> +    type: object
> >> +    description:
> >> +      Description of one of the outputs (OUT1..OUT4). See "Clock1 Output
> >> +      Configuration" in the Versaclock 5/6/6E Family Register Description
> >> +      and Programming Guide.
> >> +    properties:
> >> +      idt,mode:
> >> +        description:
> >> +          The output drive mode. Values defined in dt-bindings/clk/versaclock.h
> >> +        enum:
> >> +          - VC5_LVPECL
> >
> > This is defining a string. Can't use defines here.
>
> How do I use the defines from include/dt-bindings then? Or should I just
> use the numeric values then, like:
>
>   idt,mode:
>     description:
>       The output drive mode. Values defined in
>       dt-bindings/clk/versaclock.h
>     minimum: 0
>     maximum: 6
>
> ?

Yes.

>
> >> +      idt,voltage-microvolts:
> >> +        description: The output drive voltage.
> >> +        $ref: /schemas/types.yaml#/definitions/uint32
> >
> > Standard unit suffixes have a type already, so drop.
>
> Ok.
>
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            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
> >> +      properties:
> >> +        clock-names:
> >> +          anyOf:
> >> +            - required: [ xin ]
> >> +            - required: [ clkin ]
> >
> > This isn't valid. I think you want:
> >
> > clock-names:
> >   minItems: 1
> >   items:
> >     - const: xin
> >     - const: clkin
> >
> > This would mean 'xin' is always required, clkin is optional.
>
> No, what I wanted to mean is that allowed cases are:
>  * for idt,5p49v5933 and idt,5p49v5935:
>    - only 'xin' (required)

For this you need an if/then schema. There are plenty of examples in
the tree, but this is what you need:

if:
  properties:
    compatible:
      enum:
        - idt,5p49v5933
        - idt,5p49v5935
then:
  properties:
    clocks:
      maxItems: 1
    clock-names:
      const: xin

>  * for the other parts one of these:
>    - only 'xin'
>    - only 'clkin'
>    - both 'xin' and 'clkin'
>
> How do I express that?

For the 2nd part:

clock-names:
  minItems: 1
  maxItems: 2
  items:
    enum: [ xin, clkin ]

> A general note: as a newcomer to yaml bindings I found a steep learning
> curve. Finding a correct construct (not to mention the best one) for
> each situation is time consuming and frustrating. I've been looking at
> existing files for suitable examples but it doesn't work very well.
>
> Is there any guide to yaml bindings for beginners with examples of
> typical cases? It would greatly help in producing better patches and
> saving time for everybody.

bindings/example-schema.yaml is intended to do that. No doubt it could
use more examples. Though from my perspective people already don't
read and follow things it says there.

The problem I think is not the typical cases, but the atypical ones. I
don't think we can enumerate all the atypical cases. At that point you
really need some understanding of json-schema which I agree has a bit
of a learning curve if you've never used anything like it (I hadn't).

Rob