The description property was located where it applies to every port, not
just ports 5 or 6 (CPU ports). Fix this description.
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
---
v2 -> v3
* New patch.
---
.../bindings/net/dsa/mediatek,mt7530.yaml | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 7df4ea1901ce..415e6c40787e 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -156,17 +156,6 @@ patternProperties:
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:
@@ -174,6 +163,9 @@ patternProperties:
then:
properties:
reg:
+ description:
+ Port address described must be 5 or 6 for CPU port and from
+ 0 to 5 for user ports
enum:
- 5
- 6
--
2.25.1
On 28.11.2022 01:47, Colin Foster wrote: > The description property was located where it applies to every port, not > just ports 5 or 6 (CPU ports). Fix this description. I'm not sure I understand. The description for reg does apply to every port. Both CPU ports and user ports are described. This patch moves the description to under CPU ports only. > > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > --- > > v2 -> v3 > * New patch. > > --- > .../bindings/net/dsa/mediatek,mt7530.yaml | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > index 7df4ea1901ce..415e6c40787e 100644 > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > @@ -156,17 +156,6 @@ patternProperties: > > patternProperties: > "^(ethernet-)?port@[0-9]+$": > - type: object > - description: Ethernet switch ports > - > - unevaluatedProperties: false > - Would be nice to mention these being removed on the patch log. Or remove them while doing ("dt-bindings: net: dsa: utilize base definitions for standard dsa switches"). > - 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: > @@ -174,6 +163,9 @@ patternProperties: > then: > properties: > reg: > + description: > + Port address described must be 5 or 6 for CPU port and from > + 0 to 5 for user ports > enum: > - 5 > - 6 Thank you for your efforts. Arınç
Hi Arınç, On Mon, Nov 28, 2022 at 11:28:31AM +0300, Arınç ÜNAL wrote: > On 28.11.2022 01:47, Colin Foster wrote: > > The description property was located where it applies to every port, not > > just ports 5 or 6 (CPU ports). Fix this description. > > I'm not sure I understand. The description for reg does apply to every port. > Both CPU ports and user ports are described. This patch moves the > description to under CPU ports only. You're right. I misinterpreted what Rob suggested, so the commit message isn't correct. I see now that reg applies to every port, but is only restricted for CPU ports (if: required: [ ethernet ]). I'll clean this message up. > > > > > Suggested-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > > --- > > > > v2 -> v3 > > * New patch. > > > > --- > > .../bindings/net/dsa/mediatek,mt7530.yaml | 14 +++----------- > > 1 file changed, 3 insertions(+), 11 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > index 7df4ea1901ce..415e6c40787e 100644 > > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml > > @@ -156,17 +156,6 @@ patternProperties: > > patternProperties: > > "^(ethernet-)?port@[0-9]+$": > > - type: object > > - description: Ethernet switch ports > > - > > - unevaluatedProperties: false > > - > > Would be nice to mention these being removed on the patch log. Or remove > them while doing ("dt-bindings: net: dsa: utilize base definitions for > standard dsa switches"). Agreed. My gut is telling me this wants to be in a separate patch from the generic DSA base definitions patch... but I can't say why for certain. I'll plan to move these to the patch you suggest and add a comment in there about how the type, description, and unevaluatedProperties of mediatek,mt7530 is no longer needed as a result? Keep this patch as more of a "restrict custom port description to CPU ports only" patch? I also see that ("dt-bindings: net: dsa: mediatek,mt7530: remove unnecessary dsa-port reference") should probably be earlier in this patch set. I'll plan to move that earlier in the series, before ("dt-bindings: net: dsa: utilize base definitions for standard dsa switches"). > > > - 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: > > @@ -174,6 +163,9 @@ patternProperties: > > then: > > properties: > > reg: > > + description: > > + Port address described must be 5 or 6 for CPU port and from > > + 0 to 5 for user ports > > enum: > > - 5 > > - 6 > > Thank you for your efforts. I greatly appreciate your help and feedback! > > Arınç
Hey Colin, On 29.11.2022 09:32, Colin Foster wrote: > Hi Arınç, > > On Mon, Nov 28, 2022 at 11:28:31AM +0300, Arınç ÜNAL wrote: >> On 28.11.2022 01:47, Colin Foster wrote: >>> The description property was located where it applies to every port, not >>> just ports 5 or 6 (CPU ports). Fix this description. >> >> I'm not sure I understand. The description for reg does apply to every port. >> Both CPU ports and user ports are described. This patch moves the >> description to under CPU ports only. > > You're right. I misinterpreted what Rob suggested, so the commit message > isn't correct. I see now that reg applies to every port, but is only > restricted for CPU ports (if: required: [ ethernet ]). I'll clean this > message up. > >> >>> >>> Suggested-by: Rob Herring <robh@kernel.org> >>> Signed-off-by: Colin Foster <colin.foster@in-advantage.com> >>> --- >>> >>> v2 -> v3 >>> * New patch. >>> >>> --- >>> .../bindings/net/dsa/mediatek,mt7530.yaml | 14 +++----------- >>> 1 file changed, 3 insertions(+), 11 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> index 7df4ea1901ce..415e6c40787e 100644 >>> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml >>> @@ -156,17 +156,6 @@ patternProperties: >>> patternProperties: >>> "^(ethernet-)?port@[0-9]+$": >>> - type: object >>> - description: Ethernet switch ports >>> - >>> - unevaluatedProperties: false >>> - >> >> Would be nice to mention these being removed on the patch log. Or remove >> them while doing ("dt-bindings: net: dsa: utilize base definitions for >> standard dsa switches"). > > Agreed. My gut is telling me this wants to be in a separate patch from > the generic DSA base definitions patch... but I can't say why for > certain. I'll plan to move these to the patch you suggest and add a comment If I understand correctly, with ("dt-bindings: net: dsa: utilize base definitions for standard dsa switches"), these properties are now defined on dsa.yaml#/$defs/base and no longer needed to be defined here since mediatek,mt7530.yaml was also made to refer to it. It'd make sense to remove these properties there as there's continuity. > in there about how the type, description, and unevaluatedProperties of > mediatek,mt7530 is no longer needed as a result? Keep this patch as more > of a "restrict custom port description to CPU ports only" patch? I'd say get rid of this patch and do above. Trust your gut though. ;P Arınç
© 2016 - 2024 Red Hat, Inc.