| Message ID | 20230406080141.22924-3-arinc.unal@arinc9.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] dt-bindings: net: dsa: mediatek,mt7530: correct brand name | expand |
On Thu, Apr 06, 2023 at 11:01:37AM +0300, arinc9.unal@gmail.com wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > The switch on MT7988 has got only port 6 as a CPU port. The only phy-mode > to be used is internal. Add this. > > Some bindings are incorrect for this switch now, so move them to more > specific places. > > Address the incorrect information of which ports can be used as a user > port. Any port can be used as a user port. > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> Acked-by: Daniel Golle <daniel@makrotopia.org> > --- > .../bindings/net/dsa/mediatek,mt7530.yaml | 63 ++++++++++++++----- > 1 file changed, 46 insertions(+), 17 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index 7045a98d9593..605888ce2bc6 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -160,22 +160,6 @@ patternProperties: > "^(ethernet-)?port@[0-9]+$": > type: object > > - properties: > - reg: > - description: > - Port address described must be 5 or 6 for CPU port and from 0 to 5 > - for user ports. > - > - allOf: > - - if: > - required: [ ethernet ] > - then: > - properties: > - reg: > - enum: > - - 5 > - - 6 > - > required: > - compatible > - reg > @@ -186,9 +170,21 @@ $defs: > "^(ethernet-)?ports$": > patternProperties: > "^(ethernet-)?port@[0-9]+$": > + properties: > + reg: > + description: > + Port address described must be 5 or 6 for the CPU port. User > + ports can be 0 to 6. > + > if: > required: [ ethernet ] > then: > + properties: > + reg: > + enum: > + - 5 > + - 6 > + > if: > properties: > reg: > @@ -212,9 +208,21 @@ $defs: > "^(ethernet-)?ports$": > patternProperties: > "^(ethernet-)?port@[0-9]+$": > + properties: > + reg: > + description: > + Port address described must be 5 or 6 for the CPU port. User > + ports can be 0 to 6. > + > if: > required: [ ethernet ] > then: > + properties: > + reg: > + enum: > + - 5 > + - 6 > + > if: > properties: > reg: > @@ -235,6 +243,27 @@ $defs: > - 2500base-x > - sgmii > > + mt7988-dsa-port: > + patternProperties: > + "^(ethernet-)?ports$": > + patternProperties: > + "^(ethernet-)?port@[0-9]+$": > + properties: > + reg: > + description: > + Port address described must be 6 for the CPU port. User ports > + can be 0 to 3, and 6. > + > + if: > + required: [ ethernet ] > + then: > + properties: > + reg: > + const: 6 > + > + phy-mode: > + const: internal > + > allOf: > - $ref: dsa.yaml#/$defs/ethernet-ports > - if: > @@ -285,7 +314,7 @@ allOf: > compatible: > const: mediatek,mt7988-switch > then: > - $ref: "#/$defs/mt7530-dsa-port" > + $ref: "#/$defs/mt7988-dsa-port" > properties: > gpio-controller: false > mediatek,mcm: false > -- > 2.37.2 >
On 06/04/2023 10:01, arinc9.unal@gmail.com wrote: > From: Arınç ÜNAL <arinc.unal@arinc9.com> > > The switch on MT7988 has got only port 6 as a CPU port. The only phy-mode > to be used is internal. Add this. > > Some bindings are incorrect for this switch now, so move them to more > specific places. > > Address the incorrect information of which ports can be used as a user > port. Any port can be used as a user port. > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> > --- > .../bindings/net/dsa/mediatek,mt7530.yaml | 63 ++++++++++++++----- > 1 file changed, 46 insertions(+), 17 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index 7045a98d9593..605888ce2bc6 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -160,22 +160,6 @@ patternProperties: > "^(ethernet-)?port@[0-9]+$": > type: object > > - properties: > - reg: > - description: > - Port address described must be 5 or 6 for CPU port and from 0 to 5 > - for user ports. > - > - allOf: > - - if: > - required: [ ethernet ] > - then: > - properties: > - reg: > - enum: > - - 5 > - - 6 > - I have doubts that the binding is still maintainable/reviewable. First, why do you need all above patterns after removal of entire contents? Second, amount of if-then-if-then located in existing blocks (not top-level) is quite big. I counted if-then-using defs, where defs has patternProps-patternProps-if-then-if-then-properties.... OMG. :) Best regards, Krzysztof
On 6.04.2023 22:07, Krzysztof Kozlowski wrote: > On 06/04/2023 10:01, arinc9.unal@gmail.com wrote: >> From: Arınç ÜNAL <arinc.unal@arinc9.com> >> >> The switch on MT7988 has got only port 6 as a CPU port. The only phy-mode >> to be used is internal. Add this. >> >> Some bindings are incorrect for this switch now, so move them to more >> specific places. >> >> Address the incorrect information of which ports can be used as a user >> port. Any port can be used as a user port. >> >> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >> --- >> .../bindings/net/dsa/mediatek,mt7530.yaml | 63 ++++++++++++++----- >> 1 file changed, 46 insertions(+), 17 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >> index 7045a98d9593..605888ce2bc6 100644 >> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >> @@ -160,22 +160,6 @@ patternProperties: >> "^(ethernet-)?port@[0-9]+$": >> type: object >> >> - properties: >> - reg: >> - description: >> - Port address described must be 5 or 6 for CPU port and from 0 to 5 >> - for user ports. >> - >> - allOf: >> - - if: >> - required: [ ethernet ] >> - then: >> - properties: >> - reg: >> - enum: >> - - 5 >> - - 6 >> - > > I have doubts that the binding is still maintainable/reviewable. First, > why do you need all above patterns after removal of entire contents? The 'type: object' item is still globally used. I'd have to define that on each definitions, I suppose? > > Second, amount of if-then-if-then located in existing blocks (not > top-level) is quite big. I counted if-then-using defs, where defs has > patternProps-patternProps-if-then-if-then-properties.... OMG. :) Yup, not much to do if we want to keep the information. I'm still maintaining this though. ¯\_(ツ)_/¯ Arınç
On 06/04/2023 21:18, Arınç ÜNAL wrote: > On 6.04.2023 22:07, Krzysztof Kozlowski wrote: >> On 06/04/2023 10:01, arinc9.unal@gmail.com wrote: >>> From: Arınç ÜNAL <arinc.unal@arinc9.com> >>> >>> The switch on MT7988 has got only port 6 as a CPU port. The only phy-mode >>> to be used is internal. Add this. >>> >>> Some bindings are incorrect for this switch now, so move them to more >>> specific places. >>> >>> Address the incorrect information of which ports can be used as a user >>> port. Any port can be used as a user port. >>> >>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>> --- >>> .../bindings/net/dsa/mediatek,mt7530.yaml | 63 ++++++++++++++----- >>> 1 file changed, 46 insertions(+), 17 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> index 7045a98d9593..605888ce2bc6 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> @@ -160,22 +160,6 @@ patternProperties: >>> "^(ethernet-)?port@[0-9]+$": >>> type: object >>> >>> - properties: >>> - reg: >>> - description: >>> - Port address described must be 5 or 6 for CPU port and from 0 to 5 >>> - for user ports. >>> - >>> - allOf: >>> - - if: >>> - required: [ ethernet ] >>> - then: >>> - properties: >>> - reg: >>> - enum: >>> - - 5 >>> - - 6 >>> - >> >> I have doubts that the binding is still maintainable/reviewable. First, >> why do you need all above patterns after removal of entire contents? > > The 'type: object' item is still globally used. I'd have to define that > on each definitions, I suppose? Doesn't it come from dsa.yaml/dsa-port.yaml schema? > >> >> Second, amount of if-then-if-then located in existing blocks (not >> top-level) is quite big. I counted if-then-using defs, where defs has >> patternProps-patternProps-if-then-if-then-properties.... OMG. :) > > Yup, not much to do if we want to keep the information. I'm still > maintaining this though. ¯\_(ツ)_/¯ Maybe it should be split into few bindings sharing common part. Best regards, Krzysztof
On 7.04.2023 12:07, Krzysztof Kozlowski wrote: > On 06/04/2023 21:18, Arınç ÜNAL wrote: >> On 6.04.2023 22:07, Krzysztof Kozlowski wrote: >>> On 06/04/2023 10:01, arinc9.unal@gmail.com wrote: >>>> From: Arınç ÜNAL <arinc.unal@arinc9.com> >>>> >>>> The switch on MT7988 has got only port 6 as a CPU port. The only phy-mode >>>> to be used is internal. Add this. >>>> >>>> Some bindings are incorrect for this switch now, so move them to more >>>> specific places. >>>> >>>> Address the incorrect information of which ports can be used as a user >>>> port. Any port can be used as a user port. >>>> >>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>>> --- >>>> .../bindings/net/dsa/mediatek,mt7530.yaml | 63 ++++++++++++++----- >>>> 1 file changed, 46 insertions(+), 17 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>> index 7045a98d9593..605888ce2bc6 100644 >>>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>> @@ -160,22 +160,6 @@ patternProperties: >>>> "^(ethernet-)?port@[0-9]+$": >>>> type: object >>>> >>>> - properties: >>>> - reg: >>>> - description: >>>> - Port address described must be 5 or 6 for CPU port and from 0 to 5 >>>> - for user ports. >>>> - >>>> - allOf: >>>> - - if: >>>> - required: [ ethernet ] >>>> - then: >>>> - properties: >>>> - reg: >>>> - enum: >>>> - - 5 >>>> - - 6 >>>> - >>> >>> I have doubts that the binding is still maintainable/reviewable. First, >>> why do you need all above patterns after removal of entire contents? >> >> The 'type: object' item is still globally used. I'd have to define that >> on each definitions, I suppose? > > Doesn't it come from dsa.yaml/dsa-port.yaml schema? It comes from dsa.yaml#/$defs/ethernet-ports which this schema already refers to. I'll remove the patterns above. Though 'type: object' is not there for "^(ethernet-)?port@[0-9]+$". I think I should add it there as the dsa-port.yaml schema defines the properties of the DSA switch port object. So the value matching the "^(ethernet-)?port@[0-9]+$" regular expression is expected to be an object conforming to the structure defined in dsa-port.yaml. Does that make sense? > >> >>> >>> Second, amount of if-then-if-then located in existing blocks (not >>> top-level) is quite big. I counted if-then-using defs, where defs has >>> patternProps-patternProps-if-then-if-then-properties.... OMG. :) >> >> Yup, not much to do if we want to keep the information. I'm still >> maintaining this though. ¯\_(ツ)_/¯ > > Maybe it should be split into few bindings sharing common part. Agreed, I think it makes sense to split this to MT7530, MT7531, and MT7988. I will do this after this series. Arınç
On 07/04/2023 11:46, Arınç ÜNAL wrote: > On 7.04.2023 12:07, Krzysztof Kozlowski wrote: >> On 06/04/2023 21:18, Arınç ÜNAL wrote: >>> On 6.04.2023 22:07, Krzysztof Kozlowski wrote: >>>> On 06/04/2023 10:01, arinc9.unal@gmail.com wrote: >>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com> >>>>> >>>>> The switch on MT7988 has got only port 6 as a CPU port. The only phy-mode >>>>> to be used is internal. Add this. >>>>> >>>>> Some bindings are incorrect for this switch now, so move them to more >>>>> specific places. >>>>> >>>>> Address the incorrect information of which ports can be used as a user >>>>> port. Any port can be used as a user port. >>>>> >>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>>>> --- >>>>> .../bindings/net/dsa/mediatek,mt7530.yaml | 63 ++++++++++++++----- >>>>> 1 file changed, 46 insertions(+), 17 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>>> index 7045a98d9593..605888ce2bc6 100644 >>>>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>>> @@ -160,22 +160,6 @@ patternProperties: >>>>> "^(ethernet-)?port@[0-9]+$": >>>>> type: object >>>>> >>>>> - properties: >>>>> - reg: >>>>> - description: >>>>> - Port address described must be 5 or 6 for CPU port and from 0 to 5 >>>>> - for user ports. >>>>> - >>>>> - allOf: >>>>> - - if: >>>>> - required: [ ethernet ] >>>>> - then: >>>>> - properties: >>>>> - reg: >>>>> - enum: >>>>> - - 5 >>>>> - - 6 >>>>> - >>>> >>>> I have doubts that the binding is still maintainable/reviewable. First, >>>> why do you need all above patterns after removal of entire contents? >>> >>> The 'type: object' item is still globally used. I'd have to define that >>> on each definitions, I suppose? >> >> Doesn't it come from dsa.yaml/dsa-port.yaml schema? > > It comes from dsa.yaml#/$defs/ethernet-ports which this schema already > refers to. I'll remove the patterns above. > > Though 'type: object' is not there for "^(ethernet-)?port@[0-9]+$". I > think I should add it there as the dsa-port.yaml schema defines the > properties of the DSA switch port object. It has ref, which is enough. > So the value matching the > "^(ethernet-)?port@[0-9]+$" regular expression is expected to be an > object conforming to the structure defined in dsa-port.yaml. > > Does that make sense? Hm, no, sorry, I still do not see what exactly is missing from dsa.yaml/port that you need to define here. Best regards, Krzysztof
On 12.04.2023 19:39, Krzysztof Kozlowski wrote: > On 07/04/2023 11:46, Arınç ÜNAL wrote: >> On 7.04.2023 12:07, Krzysztof Kozlowski wrote: >>> On 06/04/2023 21:18, Arınç ÜNAL wrote: >>>> On 6.04.2023 22:07, Krzysztof Kozlowski wrote: >>>>> On 06/04/2023 10:01, arinc9.unal@gmail.com wrote: >>>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com> >>>>>> >>>>>> The switch on MT7988 has got only port 6 as a CPU port. The only phy-mode >>>>>> to be used is internal. Add this. >>>>>> >>>>>> Some bindings are incorrect for this switch now, so move them to more >>>>>> specific places. >>>>>> >>>>>> Address the incorrect information of which ports can be used as a user >>>>>> port. Any port can be used as a user port. >>>>>> >>>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >>>>>> --- >>>>>> .../bindings/net/dsa/mediatek,mt7530.yaml | 63 ++++++++++++++----- >>>>>> 1 file changed, 46 insertions(+), 17 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>>>> index 7045a98d9593..605888ce2bc6 100644 >>>>>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>>>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>>>>> @@ -160,22 +160,6 @@ patternProperties: >>>>>> "^(ethernet-)?port@[0-9]+$": >>>>>> type: object >>>>>> >>>>>> - properties: >>>>>> - reg: >>>>>> - description: >>>>>> - Port address described must be 5 or 6 for CPU port and from 0 to 5 >>>>>> - for user ports. >>>>>> - >>>>>> - allOf: >>>>>> - - if: >>>>>> - required: [ ethernet ] >>>>>> - then: >>>>>> - properties: >>>>>> - reg: >>>>>> - enum: >>>>>> - - 5 >>>>>> - - 6 >>>>>> - >>>>> >>>>> I have doubts that the binding is still maintainable/reviewable. First, >>>>> why do you need all above patterns after removal of entire contents? >>>> >>>> The 'type: object' item is still globally used. I'd have to define that >>>> on each definitions, I suppose? >>> >>> Doesn't it come from dsa.yaml/dsa-port.yaml schema? >> >> It comes from dsa.yaml#/$defs/ethernet-ports which this schema already >> refers to. I'll remove the patterns above. >> >> Though 'type: object' is not there for "^(ethernet-)?port@[0-9]+$". I >> think I should add it there as the dsa-port.yaml schema defines the >> properties of the DSA switch port object. > > It has ref, which is enough. > >> So the value matching the >> "^(ethernet-)?port@[0-9]+$" regular expression is expected to be an >> object conforming to the structure defined in dsa-port.yaml. >> >> Does that make sense? > > Hm, no, sorry, I still do not see what exactly is missing from > dsa.yaml/port that you need to define here. Nothing, I forgot defining either ref or type is enough. Arınç
diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml index 7045a98d9593..605888ce2bc6 100644 --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml @@ -160,22 +160,6 @@ patternProperties: "^(ethernet-)?port@[0-9]+$": type: object - properties: - reg: - description: - Port address described must be 5 or 6 for CPU port and from 0 to 5 - for user ports. - - allOf: - - if: - required: [ ethernet ] - then: - properties: - reg: - enum: - - 5 - - 6 - required: - compatible - reg @@ -186,9 +170,21 @@ $defs: "^(ethernet-)?ports$": patternProperties: "^(ethernet-)?port@[0-9]+$": + properties: + reg: + description: + Port address described must be 5 or 6 for the CPU port. User + ports can be 0 to 6. + if: required: [ ethernet ] then: + properties: + reg: + enum: + - 5 + - 6 + if: properties: reg: @@ -212,9 +208,21 @@ $defs: "^(ethernet-)?ports$": patternProperties: "^(ethernet-)?port@[0-9]+$": + properties: + reg: + description: + Port address described must be 5 or 6 for the CPU port. User + ports can be 0 to 6. + if: required: [ ethernet ] then: + properties: + reg: + enum: + - 5 + - 6 + if: properties: reg: @@ -235,6 +243,27 @@ $defs: - 2500base-x - sgmii + mt7988-dsa-port: + patternProperties: + "^(ethernet-)?ports$": + patternProperties: + "^(ethernet-)?port@[0-9]+$": + properties: + reg: + description: + Port address described must be 6 for the CPU port. User ports + can be 0 to 3, and 6. + + if: + required: [ ethernet ] + then: + properties: + reg: + const: 6 + + phy-mode: + const: internal + allOf: - $ref: dsa.yaml#/$defs/ethernet-ports - if: @@ -285,7 +314,7 @@ allOf: compatible: const: mediatek,mt7988-switch then: - $ref: "#/$defs/mt7530-dsa-port" + $ref: "#/$defs/mt7988-dsa-port" properties: gpio-controller: false mediatek,mcm: false