Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema

[Arınç ÜNAL]

On 12.08.2022 10:01, Krzysztof Kozlowski wrote:
> On 12/08/2022 09:57, Krzysztof Kozlowski wrote:
> > On 12/08/2022 01:09, Arınç ÜNAL wrote:
> > > > > -patternProperties:
> > > > > -  "^(ethernet-)?ports$":
> > > > > -    type: object
> > > >
> > > > Actually four patches...
> > > >
> > > > I don't find this change explained in commit msg. What is more, it looks
> > > > incorrect. All properties and patternProperties should be explained in
> > > > top-level part.
> > > >
> > > > Defining such properties (with big piece of YAML) in each if:then: is no
> > > > readable.
> > >
> > > I can't figure out another way. I need to require certain properties for
> > > a compatible string AND certain enum/const for certain properties which
> > > are inside patternProperties for "^(ethernet-)?port@[0-9]+$" by reading
> > > the compatible string.
> >
> > requiring properties is not equal to defining them and nothing stops you
> > from defining all properties top-level and requiring them in
> > allOf:if:then:patternProperties.
> >
> >
> > > If I put allOf:if:then under patternProperties, I can't do the latter.
> >
> > You can.

Am I supposed to do something like this:

patternProperties:
  "^(ethernet-)?ports$":
    type: object

    patternProperties:
      "^(ethernet-)?port@[0-9]+$":
        type: object
        description: Ethernet switch ports

        unevaluatedProperties: false

        properties:
          reg:
            description:
              Port address described must be 5 or 6 for CPU port and
              from 0 to 5 for user ports.

        allOf:
          - $ref: dsa-port.yaml#
          - if:
              properties:
                label:
                  items:
                    - const: cpu
            then:
              allOf:
                - if:
                    properties:
                      compatible:
                        items:
                          - const: mediatek,mt7530
                          - const: mediatek,mt7621
                  then:
                    allOf:
                      - if:
                          properties:
                            reg:
                              const: 5
                        then:
                          properties:
                            phy-mode:
                              enum:
                                - gmii
                                - mii
                                - rgmii

                      - if:
                          properties:
                            reg:
                              const: 6
                        then:
                          properties:
                            phy-mode:
                              enum:
                                - rgmii
                                - trgmii

                - if:
                    properties:
                      compatible:
                        items:
                          - const: mediatek,mt7531
                  then:
                    allOf:
                      - if:
                          properties:
                            reg:
                              const: 5
                        then:
                          properties:
                            phy-mode:
                              enum:
                                - 1000base-x
                                - 2500base-x
                                - rgmii
                                - sgmii

                      - if:
                          properties:
                            reg:
                              const: 6
                        then:
                          properties:
                            phy-mode:
                              enum:
                                - 1000base-x
                                - 2500base-x
                                - sgmii

              properties:
                reg:
                  enum:
                    - 5
                    - 6

              required:
                - phy-mode

> > > Other than readability to human eyes, binding check works as intended,
> > > in case there's no other way to do it.
> >
> > I don't see the problem in doing it and readability is one of main
> > factors of code admission to Linux kernel.
>
> One more thought - if your schema around allOf:if:then grows too much,
> it is actually a sign that it might benefit from splitting. Either into
> two separate schemas or into common+two separate.
>
> Best regards,
> Krzysztof

Arınç