| Message ID | 20220820080758.9829-5-arinc.unal@arinc9.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | completely rework mediatek,mt7530 binding | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 20/08/2022 11:07, Arınç ÜNAL wrote: > Define DSA port binding per switch model as each switch model requires > different values for certain properties. > > Define reg property on $defs as it's the same for all switch models. > > Remove unnecessary lines as they are already included from the referred > dsa.yaml. > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- > .../bindings/net/dsa/mediatek,mt7530.yaml | 56 +++++++++++-------- > 1 file changed, 34 insertions(+), 22 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index 657e162a1c01..7c4374e16f96 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -130,38 +130,47 @@ properties: > ethsys. > maxItems: 1 > > -patternProperties: > - "^(ethernet-)?ports$": > - type: object > - > - patternProperties: > - "^(ethernet-)?port@[0-9]+$": > - type: object > - description: Ethernet switch ports Again, I don't understand why do you remove definitions of these nodes from top-level properties. I explained what I expect in previous discussion and I am confused to hear "this cannot be done". Best regards, Krzysztof
On 23.08.2022 13:47, Krzysztof Kozlowski wrote: > On 20/08/2022 11:07, Arınç ÜNAL wrote: >> Define DSA port binding per switch model as each switch model requires >> different values for certain properties. >> >> Define reg property on $defs as it's the same for all switch models. >> >> Remove unnecessary lines as they are already included from the referred >> dsa.yaml. >> >> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >> --- >> .../bindings/net/dsa/mediatek,mt7530.yaml | 56 +++++++++++-------- >> 1 file changed, 34 insertions(+), 22 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >> index 657e162a1c01..7c4374e16f96 100644 >> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >> @@ -130,38 +130,47 @@ properties: >> ethsys. >> maxItems: 1 >> >> -patternProperties: >> - "^(ethernet-)?ports$": >> - type: object >> - >> - patternProperties: >> - "^(ethernet-)?port@[0-9]+$": >> - type: object >> - description: Ethernet switch ports > > Again, I don't understand why do you remove definitions of these nodes > from top-level properties. I explained what I expect in previous > discussion and I am confused to hear "this cannot be done". I agree it can be done, but the binding is done with less lines the current way. I would need to add more lines than just for creating the node structure since dsa.yaml is not referred. Then, I would have to create the node structure again for the dsa-port checks. Arınç
On 23/08/2022 15:29, Arınç ÜNAL wrote: > > > On 23.08.2022 13:47, Krzysztof Kozlowski wrote: >> On 20/08/2022 11:07, Arınç ÜNAL wrote: >>> Define DSA port binding per switch model as each switch model requires >>> different values for certain properties. >>> >>> Define reg property on $defs as it's the same for all switch models. >>> >>> Remove unnecessary lines as they are already included from the referred >>> dsa.yaml. >>> >>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>> --- >>> .../bindings/net/dsa/mediatek,mt7530.yaml | 56 +++++++++++-------- >>> 1 file changed, 34 insertions(+), 22 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> index 657e162a1c01..7c4374e16f96 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> @@ -130,38 +130,47 @@ properties: >>> ethsys. >>> maxItems: 1 >>> >>> -patternProperties: >>> - "^(ethernet-)?ports$": >>> - type: object >>> - >>> - patternProperties: >>> - "^(ethernet-)?port@[0-9]+$": >>> - type: object >>> - description: Ethernet switch ports >> >> Again, I don't understand why do you remove definitions of these nodes >> from top-level properties. I explained what I expect in previous >> discussion and I am confused to hear "this cannot be done". > > I agree it can be done, but the binding is done with less lines the > current way. > > I would need to add more lines than just for creating the node structure > since dsa.yaml is not referred. > > Then, I would have to create the node structure again for the dsa-port > checks. I understand you can create binding more concise, but not necessarily more readable. The easiest to grasp is to define all the nodes in top-level and customize them in allOf:if:then. This was actually also needed for earlier dtschema with additionalProperties:false. You keep defining properties in allOf:if:then, even though they are all applicable to all variants. That's unusual and even if it reduces the lines does not make it easier to grasp. Best regards, Krzysztof
On 25.08.2022 09:33, Krzysztof Kozlowski wrote: > On 23/08/2022 15:29, Arınç ÜNAL wrote: >> >> >> On 23.08.2022 13:47, Krzysztof Kozlowski wrote: >>> On 20/08/2022 11:07, Arınç ÜNAL wrote: >>>> Define DSA port binding per switch model as each switch model requires >>>> different values for certain properties. >>>> >>>> Define reg property on $defs as it's the same for all switch models. >>>> >>>> Remove unnecessary lines as they are already included from the referred >>>> dsa.yaml. >>>> >>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>>> --- >>>> .../bindings/net/dsa/mediatek,mt7530.yaml | 56 +++++++++++-------- >>>> 1 file changed, 34 insertions(+), 22 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>> index 657e162a1c01..7c4374e16f96 100644 >>>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>> @@ -130,38 +130,47 @@ properties: >>>> ethsys. >>>> maxItems: 1 >>>> >>>> -patternProperties: >>>> - "^(ethernet-)?ports$": >>>> - type: object >>>> - >>>> - patternProperties: >>>> - "^(ethernet-)?port@[0-9]+$": >>>> - type: object >>>> - description: Ethernet switch ports >>> >>> Again, I don't understand why do you remove definitions of these nodes >>> from top-level properties. I explained what I expect in previous >>> discussion and I am confused to hear "this cannot be done". >> >> I agree it can be done, but the binding is done with less lines the >> current way. >> >> I would need to add more lines than just for creating the node structure >> since dsa.yaml is not referred. >> >> Then, I would have to create the node structure again for the dsa-port >> checks. > > I understand you can create binding more concise, but not necessarily > more readable. The easiest to grasp is to define all the nodes in > top-level and customize them in allOf:if:then. This was actually also > needed for earlier dtschema with additionalProperties:false. You keep > defining properties in allOf:if:then, even though they are all > applicable to all variants. That's unusual and even if it reduces the > lines does not make it easier to grasp. Understood. Will send v6 with respect to this. Arınç
diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml index 657e162a1c01..7c4374e16f96 100644 --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml @@ -130,38 +130,47 @@ properties: ethsys. maxItems: 1 -patternProperties: - "^(ethernet-)?ports$": - type: object - - patternProperties: - "^(ethernet-)?port@[0-9]+$": - type: object - description: Ethernet switch ports - - unevaluatedProperties: false +required: + - compatible + - reg - properties: - reg: - description: - Port address described must be 5 or 6 for CPU port and from 0 - to 5 for user ports. +$defs: + dsa-port-reg: + 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: + mt7530-dsa-port: + patternProperties: + "^(ethernet-)?ports$": + patternProperties: + "^(ethernet-)?port@[0-9]+$": + $ref: "#/$defs/dsa-port-reg" + if: properties: label: items: - const: cpu then: required: - - reg - phy-mode -required: - - compatible - - reg + mt7531-dsa-port: + patternProperties: + "^(ethernet-)?ports$": + patternProperties: + "^(ethernet-)?port@[0-9]+$": + $ref: "#/$defs/dsa-port-reg" + if: + properties: + label: + items: + - const: cpu + then: + required: + - phy-mode allOf: - $ref: dsa.yaml# @@ -185,6 +194,7 @@ allOf: items: - const: mediatek,mt7530 then: + $ref: "#/$defs/mt7530-dsa-port" required: - core-supply - io-supply @@ -195,6 +205,7 @@ allOf: items: - const: mediatek,mt7531 then: + $ref: "#/$defs/mt7531-dsa-port" properties: mediatek,mcm: false @@ -204,6 +215,7 @@ allOf: items: - const: mediatek,mt7621 then: + $ref: "#/$defs/mt7530-dsa-port" required: - mediatek,mcm
Define DSA port binding per switch model as each switch model requires different values for certain properties. Define reg property on $defs as it's the same for all switch models. Remove unnecessary lines as they are already included from the referred dsa.yaml. Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> --- .../bindings/net/dsa/mediatek,mt7530.yaml | 56 +++++++++++-------- 1 file changed, 34 insertions(+), 22 deletions(-)