Messages in this thread | | | Date | Fri, 12 Aug 2022 17:06:57 +0300 | Subject | Re: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema | From | Arınç ÜNAL <> |
| |
On 12.08.2022 16:48, Krzysztof Kozlowski wrote: > On 12/08/2022 16:41, Arınç ÜNAL wrote: >> 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: > > Not really, this is absolutely unreadable. > > Usually the way it is handled is: > > patternProperties: > "^(ethernet-)?ports$": > type: object > > patternProperties: > "^(ethernet-)?port@[0-9]+$": > type: object > description: Ethernet switch ports > unevaluatedProperties: false > ... regular stuff follows > > allOf: > - if: > properties: > compatible: > ..... > then: > patternProperties: > "^(ethernet-)?ports$": > patternProperties: > "^(ethernet-)?port@[0-9]+$": > properties: > reg: > const: 5 > > > I admit that it is still difficult to parse, which could justify > splitting to separate schema. Anyway the point of my comment was to > define all properties in top level, not in allOf. > > allOf should be used to constrain these properties.
The problem is: - only specific values of reg are allowed if label is cpu. - only specific values of phy-mode are allowed if reg is 5 or 6.
This forces me to define properties under allOf:if:then. Splitting to separate schema (per compatible string?) wouldn't help in this case.
I can split patternProperties to two sections, but I can't directly define the reg property like you put above.
I can at least split mediatek,mt7531 to a separate schema to have less patternProperties on a single binding.
What do you think?
Arınç
| |