lkml.org 
[lkml]   [2022]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 4/4] dt-bindings: net: dsa: mediatek,mt7530: update json-schema
From


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ç

\
 
 \ /
  Last update: 2022-08-12 16:08    [W:0.052 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site