2022-11-04 04:54:02

by Colin Foster

[permalink] [raw]
Subject: [PATCH v2 net-next 0/6] dt-binding preparation for ocelot switches

Ocelot switches have the abilitiy to be used internally via
memory-mapped IO or externally via SPI or PCIe. This brings up issues
for documentation, where the same chip might be accessed internally in a
switchdev manner, or externally in a DSA configuration. This patch set
is perparation to bring DSA functionality to the VSC7512, utilizing as
much as possible with an almost identical VSC7514 chip.

During the most recent RFC for internal ethernet switch functionality to
the VSC7512, there were 10 steps laid out to adequately prepare
documentation:

https://lore.kernel.org/all/20221010174856.nd3n4soxk7zbmcm7@skbuf/

The full context is quoted below. This patch set represents steps 1-7 of
the 10 steps, with the remaining steps to likely be part of what was the
original RFC.

The first two patches are specifically rewording and fixing of the MFD
bindings. I kept them in this patch set since they might cause conflicts
with future documentation changes that will be part of the net-next
tree. I can separate them if desired.



Context:

```
To end the discussion on a constructive note, I think if I were Colin,
I would do the following, in the following order, according to what was
expressed as a constraint:

1. Reword the "driver" word out of mscc,vsc7514-switch.yaml and express
the description in terms of what the switch can do, not what the
driver can do.

2. Make qca8k.yaml have "$ref: dsa.yaml#". Remove "$ref: dsa-port.yaml#"
from the same schema.

3. Remove "- $ref: dsa-port.yaml#" from mediatek,mt7530.yaml. It doesn't
seem to be needed, since dsa.yaml also has this. We need this because
we want to make sure no one except dsa.yaml references dsa-port.yaml.

4. Move the DSA-unspecific portion from dsa.yaml into a new
ethernet-switch.yaml. What remains in dsa.yaml is "dsa,member".
The dsa.yaml schema will have "$ref: ethernet-switch.yaml#" for the
"(ethernet-)switch" node, plus its custom additions.

5. Move the DSA-unspecific portion from dsa-port.yaml into a new
ethernet-switch-port.yaml. What remains in dsa-port.yaml is:
* ethernet phandle
* link phandle
* label property
* dsa-tag-protocol property
* the constraint that CPU and DSA ports must have phylink bindings

6. The ethernet-switch.yaml will have "$ref: ethernet-switch-port.yaml#"
and "$ref: dsa-port.yaml". The dsa-port.yaml schema will *not* have
"$ref: ethernet-switch-port.yaml#", just its custom additions.
I'm not 100% on this, but I think there will be a problem if:
- dsa.yaml references ethernet-switch.yaml
- ethernet-switch.yaml references ethernet-switch-port.yaml
- dsa.yaml also references dsa-port.yaml
- dsa-port.yaml references ethernet-switch-port.yaml
because ethernet-switch-port.yaml will be referenced twice. Again,
not sure if this is a problem. If it isn't, things can be simpler,
just make dsa-port.yaml reference ethernet-switch-port.yaml, and skip
steps 2 and 3 since dsa-port.yaml containing just the DSA specifics
is no longer problematic.

7. Make mscc,vsc7514-switch.yaml have "$ref: ethernet-switch.yaml#" for
the "mscc,vsc7514-switch.yaml" compatible string. This will eliminate
its own definitions for the generic properties: $nodename and
ethernet-ports (~45 lines of code if I'm not mistaken).

8. Introduce the "mscc,vsc7512-switch" compatible string as part of
mscc,vsc7514-switch.yaml, but this will have "$ref: dsa.yaml#" (this
will have to be referenced by full path because they are in different
folders) instead of "ethernet-switch.yaml". Doing this will include
the common bindings for a switch, plus the DSA specifics.

9. Optional: rework ti,cpsw-switch.yaml, microchip,lan966x-switch.yaml,
microchip,sparx5-switch.yaml to have "$ref: ethernet-switch.yaml#"
which should reduce some duplication in existing schemas.

10. Question for future support of VSC7514 in DSA mode: how do we decide
whether to $ref: ethernet-switch.yaml or dsa.yaml? If the parent MFD
node has a compatible string similar to "mscc,vsc7512", then use DSA,
otherwise use generic ethernet-switch?
```

---

v1 -> v2
* Two MFD patches were brought into the MFD tree, so are dropped
* Add first patch 1/6 to allow DSA devices to add ports and port
properties
* Test qca8k against new dt-bindings and fix warnings. (patch 2/6)
* Add tags (patch 3/6)
* Fix vsc7514 refs and properties

---


Colin Foster (6):
dt-bindings: net: dsa: allow additional ethernet-port properties
dt-bindings: net: dsa: qca8k: utilize shared dsa.yaml
dt-bindings: net: dsa: mediatek,mt7530: remove unnecessary dsa-port
reference
dt-bindings: net: add generic ethernet-switch
dt-bindings: net: add generic ethernet-switch-port binding
dt-bindings: net: mscc,vsc7514-switch: utilize generic
ethernet-switch.yaml

.../devicetree/bindings/net/dsa/dsa-port.yaml | 27 +---------
.../devicetree/bindings/net/dsa/dsa.yaml | 26 +---------
.../bindings/net/dsa/mediatek,mt7530.yaml | 3 --
.../devicetree/bindings/net/dsa/qca8k.yaml | 18 +++----
.../bindings/net/ethernet-switch-port.yaml | 44 ++++++++++++++++
.../bindings/net/ethernet-switch.yaml | 51 +++++++++++++++++++
.../bindings/net/mscc,vsc7514-switch.yaml | 40 ++-------------
MAINTAINERS | 2 +
8 files changed, 112 insertions(+), 99 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
create mode 100644 Documentation/devicetree/bindings/net/ethernet-switch.yaml

--
2.25.1



2022-11-04 04:54:08

by Colin Foster

[permalink] [raw]
Subject: [PATCH v2 net-next 1/6] dt-bindings: net: dsa: allow additional ethernet-port properties

Explicitly allow additional properties for both the ethernet-port and
ethernet-ports properties. This specifically will allow the qca8k.yaml
binding to use shared properties.

Signed-off-by: Colin Foster <[email protected]>
---

v1 -> v2
* New patch

---
Documentation/devicetree/bindings/net/dsa/dsa.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
index b9d48e357e77..af9010c4f069 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
@@ -41,6 +41,8 @@ patternProperties:
'#size-cells':
const: 0

+ additionalProperties: true
+
patternProperties:
"^(ethernet-)?port@[0-9]+$":
type: object
@@ -48,7 +50,7 @@ patternProperties:

$ref: dsa-port.yaml#

- unevaluatedProperties: false
+ unevaluatedProperties: true

oneOf:
- required:
--
2.25.1


2022-11-04 04:55:28

by Colin Foster

[permalink] [raw]
Subject: [PATCH v2 net-next 4/6] dt-bindings: net: add generic ethernet-switch

The dsa.yaml bindings had references that can apply to non-dsa switches. To
prevent duplication of this information, keep the dsa-specific information
inside dsa.yaml and move the remaining generic information to the newly
created ethernet-switch.yaml.

Signed-off-by: Colin Foster <[email protected]>
Suggested-by: Vladimir Oltean <[email protected]>
---

v1 -> v2
* No net change, but deletions from dsa.yaml included the changes for
"addionalProperties: true" under ports and "unevaluatedProperties:
true" under port.

---
.../devicetree/bindings/net/dsa/dsa.yaml | 28 +----------
.../bindings/net/ethernet-switch.yaml | 49 +++++++++++++++++++
MAINTAINERS | 1 +
3 files changed, 52 insertions(+), 26 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ethernet-switch.yaml

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
index af9010c4f069..2290a9d32b21 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
@@ -18,6 +18,8 @@ description:

select: false

+$ref: "/schemas/net/ethernet-switch.yaml#"
+
properties:
$nodename:
pattern: "^(ethernet-)?switch(@.*)?$"
@@ -32,32 +34,6 @@ properties:
(single device hanging off a CPU port) must not specify this property
$ref: /schemas/types.yaml#/definitions/uint32-array

-patternProperties:
- "^(ethernet-)?ports$":
- type: object
- properties:
- '#address-cells':
- const: 1
- '#size-cells':
- const: 0
-
- additionalProperties: true
-
- patternProperties:
- "^(ethernet-)?port@[0-9]+$":
- type: object
- description: Ethernet switch ports
-
- $ref: dsa-port.yaml#
-
- unevaluatedProperties: true
-
-oneOf:
- - required:
- - ports
- - required:
- - ethernet-ports
-
additionalProperties: true

...
diff --git a/Documentation/devicetree/bindings/net/ethernet-switch.yaml b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
new file mode 100644
index 000000000000..fbaac536673d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ethernet-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ethernet Switch Device Tree Bindings
+
+maintainers:
+ - Andrew Lunn <[email protected]>
+ - Florian Fainelli <[email protected]>
+ - Vivien Didelot <[email protected]>
+
+description:
+ This binding represents Ethernet Switches which have a dedicated CPU
+ port. That port is usually connected to an Ethernet Controller of the
+ SoC. Such setups are typical for embedded devices.
+
+select: false
+
+properties:
+ $nodename:
+ pattern: "^(ethernet-)?switch(@.*)?$"
+
+patternProperties:
+ "^(ethernet-)?ports$":
+ type: object
+ properties:
+ '#address-cells':
+ const: 1
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ "^(ethernet-)?port@[0-9]+$":
+ type: object
+ description: Ethernet switch ports
+
+ $ref: /schemas/net/dsa/dsa-port.yaml#
+
+oneOf:
+ - required:
+ - ports
+ - required:
+ - ethernet-ports
+
+additionalProperties: true
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 3106a9f0567a..3b6c3989c419 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14326,6 +14326,7 @@ M: Florian Fainelli <[email protected]>
M: Vladimir Oltean <[email protected]>
S: Maintained
F: Documentation/devicetree/bindings/net/dsa/
+F: Documentation/devicetree/bindings/net/ethernet-switch.yaml
F: drivers/net/dsa/
F: include/linux/dsa/
F: include/linux/platform_data/dsa.h
--
2.25.1


2022-11-04 05:01:49

by Colin Foster

[permalink] [raw]
Subject: [PATCH v2 net-next 6/6] dt-bindings: net: mscc,vsc7514-switch: utilize generic ethernet-switch.yaml

Several bindings for ethernet switches are available for non-dsa switches
by way of ethernet-switch.yaml. Remove these duplicate entries and utilize
the common bindings for the VSC7514.

Signed-off-by: Colin Foster <[email protected]>
Suggested-by: Vladimir Oltean <[email protected]>
---

v1 -> v2:
* Fix "$ref: ethernet-switch.yaml" placement. Oops.
* Add "unevaluatedProperties: true" to ethernet-ports layer so it
can correctly read into ethernet-switch.yaml
* Add "unevaluatedProperties: true" to ethernet-port layer so it can
correctly read into ethernet-controller.yaml

---
.../bindings/net/mscc,vsc7514-switch.yaml | 40 ++-----------------
1 file changed, 4 insertions(+), 36 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml b/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
index ee0a504bdb24..3f3f9fd548cf 100644
--- a/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
+++ b/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
@@ -18,10 +18,9 @@ description: |
packets using CPU. Additionally, PTP is supported as well as FDMA for faster
packet extraction/injection.

-properties:
- $nodename:
- pattern: "^switch@[0-9a-f]+$"
+$ref: ethernet-switch.yaml#

+properties:
compatible:
const: mscc,vsc7514-switch

@@ -88,46 +87,15 @@ properties:
- const: fdma

ethernet-ports:
- type: object
-
- properties:
- '#address-cells':
- const: 1
- '#size-cells':
- const: 0

- additionalProperties: false
+ unevaluatedProperties: true

patternProperties:
"^port@[0-9a-f]+$":
- type: object
- description: Ethernet ports handled by the switch

$ref: ethernet-controller.yaml#

- unevaluatedProperties: false
-
- properties:
- reg:
- description: Switch port number
-
- phy-handle: true
-
- phy-mode: true
-
- fixed-link: true
-
- mac-address: true
-
- required:
- - reg
- - phy-mode
-
- oneOf:
- - required:
- - phy-handle
- - required:
- - fixed-link
+ unevaluatedProperties: true

required:
- compatible
--
2.25.1


2022-11-04 05:01:50

by Colin Foster

[permalink] [raw]
Subject: [PATCH v2 net-next 5/6] dt-bindings: net: add generic ethernet-switch-port binding

The dsa-port.yaml binding had several references that can be common to all
ethernet ports, not just dsa-specific ones. Break out the generic bindings
to ethernet-switch-port.yaml they can be used by non-dsa drivers.

Signed-off-by: Colin Foster <[email protected]>
Suggested-by: Vladimir Oltean <[email protected]>
---

v1 -> v2
* Remove accidental addition of
"$ref: /schemas/net/ethernet-switch-port.yaml" which should be kept
out of dsa-port so that it doesn't get referenced multiple times
through both ethernet-switch and dsa-port.

---
.../devicetree/bindings/net/dsa/dsa-port.yaml | 27 +-----------
.../bindings/net/ethernet-switch-port.yaml | 44 +++++++++++++++++++
.../bindings/net/ethernet-switch.yaml | 4 +-
MAINTAINERS | 1 +
4 files changed, 49 insertions(+), 27 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/ethernet-switch-port.yaml

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index 10ad7e71097b..d97fb87cccb0 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -4,7 +4,7 @@
$id: http://devicetree.org/schemas/net/dsa/dsa-port.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

-title: Ethernet Switch port Device Tree Bindings
+title: DSA Switch port Device Tree Bindings

maintainers:
- Andrew Lunn <[email protected]>
@@ -14,13 +14,7 @@ maintainers:
description:
Ethernet switch port Description

-allOf:
- - $ref: /schemas/net/ethernet-controller.yaml#
-
properties:
- reg:
- description: Port number
-
label:
description:
Describes the label associated with this port, which will become
@@ -57,25 +51,6 @@ properties:
- rtl8_4t
- seville

- phy-handle: true
-
- phy-mode: true
-
- fixed-link: true
-
- mac-address: true
-
- sfp: true
-
- managed: true
-
- rx-internal-delay-ps: true
-
- tx-internal-delay-ps: true
-
-required:
- - reg
-
# CPU and DSA ports must have phylink-compatible link descriptions
if:
oneOf:
diff --git a/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml b/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
new file mode 100644
index 000000000000..cb1e5e12bf0a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ethernet-switch-port.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ethernet Switch port Device Tree Bindings
+
+maintainers:
+ - Andrew Lunn <[email protected]>
+ - Florian Fainelli <[email protected]>
+ - Vivien Didelot <[email protected]>
+
+description:
+ Ethernet switch port Description
+
+$ref: ethernet-controller.yaml#
+
+properties:
+ reg:
+ description: Port number
+
+ phy-handle: true
+
+ phy-mode: true
+
+ fixed-link: true
+
+ mac-address: true
+
+ sfp: true
+
+ managed: true
+
+ rx-internal-delay-ps: true
+
+ tx-internal-delay-ps: true
+
+required:
+ - reg
+
+additionalProperties: true
+
+...
diff --git a/Documentation/devicetree/bindings/net/ethernet-switch.yaml b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
index fbaac536673d..f698857619da 100644
--- a/Documentation/devicetree/bindings/net/ethernet-switch.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
@@ -36,7 +36,9 @@ patternProperties:
type: object
description: Ethernet switch ports

- $ref: /schemas/net/dsa/dsa-port.yaml#
+ allOf:
+ - $ref: /schemas/net/dsa/dsa-port.yaml#
+ - $ref: ethernet-switch-port.yaml#

oneOf:
- required:
diff --git a/MAINTAINERS b/MAINTAINERS
index 3b6c3989c419..d98fc1962874 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14326,6 +14326,7 @@ M: Florian Fainelli <[email protected]>
M: Vladimir Oltean <[email protected]>
S: Maintained
F: Documentation/devicetree/bindings/net/dsa/
+F: Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
F: Documentation/devicetree/bindings/net/ethernet-switch.yaml
F: drivers/net/dsa/
F: include/linux/dsa/
--
2.25.1


2022-11-04 05:02:08

by Colin Foster

[permalink] [raw]
Subject: [PATCH v2 net-next 2/6] dt-bindings: net: dsa: qca8k: utilize shared dsa.yaml

The dsa.yaml binding contains duplicated bindings for address and size
cells, as well as the reference to dsa-port.yaml. Instead of duplicating
this information, remove the reference to dsa-port.yaml and include the
full reference to dsa.yaml.

Signed-off-by: Colin Foster <[email protected]>
Suggested-by: Vladimir Oltean <[email protected]>
---

v1 -> v2
* Add #address-cells and #size-cells to the switch layer. They aren't
part of dsa.yaml.
* Add unevaluatedProperties: true to the ethernet-port layer so it can
correctly read properties from dsa.yaml.

---
.../devicetree/bindings/net/dsa/qca8k.yaml | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index 978162df51f7..d831d5eee437 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -33,6 +33,10 @@ properties:
qca,qca8334: referenced as QCA8334-AL3C QFN 88 pin package
qca,qca8337: referenced as QCA8337N-AL3(B/C) DR-QFN 148 pin package

+ '#address-cells':
+ const: 1
+ '#size-cells':
+ const: 0
reg:
maxItems: 1

@@ -66,22 +70,16 @@ properties:
With the legacy mapping the reg corresponding to the internal
mdio is the switch reg with an offset of -1.

+$ref: "dsa.yaml#"
+
patternProperties:
"^(ethernet-)?ports$":
type: object
- properties:
- '#address-cells':
- const: 1
- '#size-cells':
- const: 0
-
patternProperties:
"^(ethernet-)?port@[0-6]$":
type: object
description: Ethernet switch ports

- $ref: dsa-port.yaml#
-
properties:
qca,sgmii-rxclk-falling-edge:
$ref: /schemas/types.yaml#/definitions/flag
@@ -104,7 +102,7 @@ patternProperties:
SGMII on the QCA8337, it is advised to set this unless a communication
issue is observed.

- unevaluatedProperties: false
+ unevaluatedProperties: true

oneOf:
- required:
@@ -116,7 +114,7 @@ required:
- compatible
- reg

-additionalProperties: true
+unevaluatedProperties: false

examples:
- |
--
2.25.1


2022-11-04 05:02:42

by Colin Foster

[permalink] [raw]
Subject: [PATCH v2 net-next 3/6] dt-bindings: net: dsa: mediatek,mt7530: remove unnecessary dsa-port reference

dsa.yaml contains a reference to dsa-port.yaml, so a duplicate reference to
the binding isn't necessary. Remove this unnecessary reference.

Signed-off-by: Colin Foster <[email protected]>
Suggested-by: Vladimir Oltean <[email protected]>
Reviewed-by: Arınç ÜNAL <[email protected]>
---

v1 -> v2
* Add Reviewed-by

---
Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 3 ---
1 file changed, 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index f2e9ff3f580b..81f291105660 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -159,8 +159,6 @@ patternProperties:
type: object
description: Ethernet switch ports

- unevaluatedProperties: false
-
properties:
reg:
description:
@@ -168,7 +166,6 @@ patternProperties:
for user ports.

allOf:
- - $ref: dsa-port.yaml#
- if:
required: [ ethernet ]
then:
--
2.25.1


2022-11-04 18:51:59

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 6/6] dt-bindings: net: mscc,vsc7514-switch: utilize generic ethernet-switch.yaml

On Thu, Nov 03, 2022 at 09:52:04PM -0700, Colin Foster wrote:
> Several bindings for ethernet switches are available for non-dsa switches
> by way of ethernet-switch.yaml. Remove these duplicate entries and utilize
> the common bindings for the VSC7514.
>
> Signed-off-by: Colin Foster <[email protected]>
> Suggested-by: Vladimir Oltean <[email protected]>
> ---
>
> v1 -> v2:
> * Fix "$ref: ethernet-switch.yaml" placement. Oops.
> * Add "unevaluatedProperties: true" to ethernet-ports layer so it
> can correctly read into ethernet-switch.yaml
> * Add "unevaluatedProperties: true" to ethernet-port layer so it can
> correctly read into ethernet-controller.yaml
>
> ---
> .../bindings/net/mscc,vsc7514-switch.yaml | 40 ++-----------------
> 1 file changed, 4 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml b/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
> index ee0a504bdb24..3f3f9fd548cf 100644
> --- a/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
> +++ b/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
> @@ -18,10 +18,9 @@ description: |
> packets using CPU. Additionally, PTP is supported as well as FDMA for faster
> packet extraction/injection.
>
> -properties:
> - $nodename:
> - pattern: "^switch@[0-9a-f]+$"
> +$ref: ethernet-switch.yaml#
>
> +properties:
> compatible:
> const: mscc,vsc7514-switch
>
> @@ -88,46 +87,15 @@ properties:
> - const: fdma
>
> ethernet-ports:
> - type: object
> -
> - properties:
> - '#address-cells':
> - const: 1
> - '#size-cells':
> - const: 0
>
> - additionalProperties: false
> + unevaluatedProperties: true

Both this and ethernet-switch.yaml allow unevaluated properties.
Therefore any extra properties will be allowed. Add some to your example
and see.

>
> patternProperties:
> "^port@[0-9a-f]+$":
> - type: object
> - description: Ethernet ports handled by the switch
>
> $ref: ethernet-controller.yaml#
>
> - unevaluatedProperties: false
> -
> - properties:
> - reg:
> - description: Switch port number
> -
> - phy-handle: true
> -
> - phy-mode: true
> -
> - fixed-link: true
> -
> - mac-address: true
> -
> - required:
> - - reg
> - - phy-mode
> -
> - oneOf:
> - - required:
> - - phy-handle
> - - required:
> - - fixed-link
> + unevaluatedProperties: true

Same problem here. I'll comment more about this on
ethernet-switch-port.yaml.

Rob

2022-11-04 19:12:16

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 5/6] dt-bindings: net: add generic ethernet-switch-port binding

On Thu, Nov 03, 2022 at 09:52:03PM -0700, Colin Foster wrote:
> The dsa-port.yaml binding had several references that can be common to all
> ethernet ports, not just dsa-specific ones. Break out the generic bindings
> to ethernet-switch-port.yaml they can be used by non-dsa drivers.
>
> Signed-off-by: Colin Foster <[email protected]>
> Suggested-by: Vladimir Oltean <[email protected]>
> ---
>
> v1 -> v2
> * Remove accidental addition of
> "$ref: /schemas/net/ethernet-switch-port.yaml" which should be kept
> out of dsa-port so that it doesn't get referenced multiple times
> through both ethernet-switch and dsa-port.
>
> ---
> .../devicetree/bindings/net/dsa/dsa-port.yaml | 27 +-----------
> .../bindings/net/ethernet-switch-port.yaml | 44 +++++++++++++++++++
> .../bindings/net/ethernet-switch.yaml | 4 +-
> MAINTAINERS | 1 +
> 4 files changed, 49 insertions(+), 27 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 10ad7e71097b..d97fb87cccb0 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -4,7 +4,7 @@
> $id: http://devicetree.org/schemas/net/dsa/dsa-port.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: Ethernet Switch port Device Tree Bindings
> +title: DSA Switch port Device Tree Bindings
>
> maintainers:
> - Andrew Lunn <[email protected]>
> @@ -14,13 +14,7 @@ maintainers:
> description:
> Ethernet switch port Description
>
> -allOf:
> - - $ref: /schemas/net/ethernet-controller.yaml#
> -
> properties:
> - reg:
> - description: Port number
> -
> label:
> description:
> Describes the label associated with this port, which will become
> @@ -57,25 +51,6 @@ properties:
> - rtl8_4t
> - seville
>
> - phy-handle: true
> -
> - phy-mode: true
> -
> - fixed-link: true
> -
> - mac-address: true
> -
> - sfp: true
> -
> - managed: true
> -
> - rx-internal-delay-ps: true
> -
> - tx-internal-delay-ps: true
> -
> -required:
> - - reg
> -
> # CPU and DSA ports must have phylink-compatible link descriptions
> if:
> oneOf:
> diff --git a/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml b/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
> new file mode 100644
> index 000000000000..cb1e5e12bf0a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ethernet-switch-port.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ethernet-switch-port.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ethernet Switch port Device Tree Bindings
> +
> +maintainers:
> + - Andrew Lunn <[email protected]>
> + - Florian Fainelli <[email protected]>
> + - Vivien Didelot <[email protected]>
> +
> +description:
> + Ethernet switch port Description
> +
> +$ref: ethernet-controller.yaml#
> +
> +properties:
> + reg:
> + description: Port number
> +
> + phy-handle: true
> +
> + phy-mode: true
> +
> + fixed-link: true
> +
> + mac-address: true
> +
> + sfp: true
> +
> + managed: true
> +
> + rx-internal-delay-ps: true
> +
> + tx-internal-delay-ps: true

I know this is just copied, but these have no effect on validation. I
assume what they are meant to be is these are the subset of
ethernet-controller.yaml which are allowed, but that would only work
with 'additionalProperties: false'. That wouldn't work because we also
want to users to extend this with custom properties. What's needed here
is a list of properties not allowed:

disallowed-prop: false

Or we can just allow anything from ethernet-controller.yaml and drop
this list.

> +
> +required:
> + - reg
> +
> +additionalProperties: true
> +
> +...

2022-11-04 19:36:24

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/6] dt-bindings: net: dsa: mediatek,mt7530: remove unnecessary dsa-port reference

On Thu, Nov 03, 2022 at 09:52:01PM -0700, Colin Foster wrote:
> dsa.yaml contains a reference to dsa-port.yaml, so a duplicate reference to
> the binding isn't necessary. Remove this unnecessary reference.
>
> Signed-off-by: Colin Foster <[email protected]>
> Suggested-by: Vladimir Oltean <[email protected]>
> Reviewed-by: Arınç ÜNAL <[email protected]>
> ---
>
> v1 -> v2
> * Add Reviewed-by
>
> ---
> Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> index f2e9ff3f580b..81f291105660 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -159,8 +159,6 @@ patternProperties:
> type: object
> description: Ethernet switch ports
>
> - unevaluatedProperties: false
> -

You just allowed this node to have any property.

> properties:
> reg:
> description:
> @@ -168,7 +166,6 @@ patternProperties:
> for user ports.
>
> allOf:
> - - $ref: dsa-port.yaml#
> - if:
> required: [ ethernet ]
> then:
> --
> 2.25.1
>
>

2022-11-04 20:15:23

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 0/6] dt-binding preparation for ocelot switches

On Thu, Nov 03, 2022 at 09:51:58PM -0700, Colin Foster wrote:
> Ocelot switches have the abilitiy to be used internally via
> memory-mapped IO or externally via SPI or PCIe. This brings up issues
> for documentation, where the same chip might be accessed internally in a
> switchdev manner, or externally in a DSA configuration. This patch set
> is perparation to bring DSA functionality to the VSC7512, utilizing as
> much as possible with an almost identical VSC7514 chip.
>
> During the most recent RFC for internal ethernet switch functionality to
> the VSC7512, there were 10 steps laid out to adequately prepare
> documentation:
>
> https://lore.kernel.org/all/20221010174856.nd3n4soxk7zbmcm7@skbuf/
>
> The full context is quoted below. This patch set represents steps 1-7 of
> the 10 steps, with the remaining steps to likely be part of what was the
> original RFC.
>
> The first two patches are specifically rewording and fixing of the MFD
> bindings. I kept them in this patch set since they might cause conflicts
> with future documentation changes that will be part of the net-next
> tree. I can separate them if desired.
>
>
>
> Context:
>
> ```
> To end the discussion on a constructive note, I think if I were Colin,
> I would do the following, in the following order, according to what was
> expressed as a constraint:
>
> 1. Reword the "driver" word out of mscc,vsc7514-switch.yaml and express
> the description in terms of what the switch can do, not what the
> driver can do.
>
> 2. Make qca8k.yaml have "$ref: dsa.yaml#". Remove "$ref: dsa-port.yaml#"
> from the same schema.

No, you need dsa-port.yaml referenced because it has DSA port properties
plus custom qca8k properties.

>
> 3. Remove "- $ref: dsa-port.yaml#" from mediatek,mt7530.yaml. It doesn't
> seem to be needed, since dsa.yaml also has this. We need this because
> we want to make sure no one except dsa.yaml references dsa-port.yaml.

You don't seem to need it in mediatek,mt7530.yaml, but only dsa.yaml
referencing dsa-port.yaml is not what we need. dsa-port.yaml wouldn't
(and didn't at one time) exist if only dsa.yaml needed it.

Something like the below patch is on top of your changes is what's
needed. For DSA, there are 2 cases, custom properties in 'port' nodes
and no custom properties. '(ethernet-)?ports' never has custom
properties AFAICT (brcm,sf2 had brcm,use-bcm-hdr in the wrong place).

Bindings using only standard DSA properties need to reference
'dsa.yaml#'. Bindings with custom 'ethernet-port' node properties need
to use 'dsa.yaml#/$defs/base' and then under the ethernet-port node
reference dsa-port.yaml, define their custom properties, and set
'unevaluatedProperties: false'.

Obviously this needs to be refactored into proper patches and/or
squashed into yours. Probably a patch to fix brcm,sf2 and one to add
dsa.yaml#/$defs/base, then split out switch stuff.

8<-------------------------------------------------------------------
diff --git a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml
index 259a0c6547f3..8d5abb05abdf 100644
--- a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Arrow SpeedChips XRS7000 Series Switch Device Tree Bindings

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base

maintainers:
- George McCollister <[email protected]>
diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
index 1219b830b1a4..f323fc01b224 100644
--- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
@@ -66,7 +66,7 @@ required:
- reg

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base
- if:
properties:
compatible:
diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml
index d159ac78cec1..eed16e216fb6 100644
--- a/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml
@@ -85,11 +85,16 @@ properties:
ports:
type: object

- properties:
- brcm,use-bcm-hdr:
- description: if present, indicates that the switch port has Broadcom
- tags enabled (per-packet metadata)
- type: boolean
+ patternProperties:
+ '^port@[0-9a-f]$':
+ $ref: dsa-port.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ brcm,use-bcm-hdr:
+ description: if present, indicates that the switch port has Broadcom
+ tags enabled (per-packet metadata)
+ type: boolean

required:
- reg
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index d97fb87cccb0..0672443ea7a6 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -14,6 +14,8 @@ maintainers:
description:
Ethernet switch port Description

+$ref: /schemas/net/ethernet-switch-port.yaml#
+
properties:
label:
description:
diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
index 2290a9d32b21..1b3593a36014 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml
@@ -21,9 +21,6 @@ select: false
$ref: "/schemas/net/ethernet-switch.yaml#"

properties:
- $nodename:
- pattern: "^(ethernet-)?switch(@.*)?$"
-
dsa,member:
minItems: 2
maxItems: 2
@@ -36,4 +33,20 @@ properties:

additionalProperties: true

+$defs:
+ base:
+ description: A DSA without any extra port properties
+ $ref: '#/'
+
+ patternProperties:
+ "^(ethernet-)?ports$":
+ type: object
+
+ patternProperties:
+ "^(ethernet-)?port@[0-9]+$":
+ description: Ethernet switch ports
+ $ref: /schemas/net/dsa/dsa-port.yaml#
+ unevaluatedProperties: false
+
+
...
diff --git a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml
index 73b774eadd0b..e27b1619066f 100644
--- a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Hirschmann Hellcreek TSN Switch Device Tree Bindings

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base

maintainers:
- Andrew Lunn <[email protected]>
diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 81f291105660..564783fcb685 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -156,21 +156,15 @@ patternProperties:

patternProperties:
"^(ethernet-)?port@[0-9]+$":
- type: object
- description: Ethernet switch ports
-
- 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:
+ description:
+ Port address described must be 5 or 6 for CPU port and from 0 to 5
+ for user ports.
enum:
- 5
- 6
@@ -235,7 +229,7 @@ $defs:
- sgmii

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base
- if:
required:
- mediatek,mcm
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
index 4da75b1f9533..bfa2b76659c9 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
@@ -11,7 +11,7 @@ maintainers:
- Woojung Huh <[email protected]>

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base
- $ref: /schemas/spi/spi-peripheral-props.yaml#

properties:
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
index 630bf0f8294b..f4f9798addae 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
@@ -10,7 +10,7 @@ maintainers:
- [email protected]

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml b/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml
index 8d93ed9c172c..a7041ae4d811 100644
--- a/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml
@@ -78,7 +78,7 @@ required:
- reg

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base
- if:
properties:
compatible:
diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
index 1e26d876d146..13a835af9468 100644
--- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
@@ -13,7 +13,7 @@ description:
depends on the SPI bus master driver.

allOf:
- - $ref: "dsa.yaml#"
+ - $ref: dsa.yaml#/$defs/base
- $ref: /schemas/spi/spi-peripheral-props.yaml#

maintainers:
diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index d831d5eee437..a33abdb9ead0 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -77,8 +77,7 @@ patternProperties:
type: object
patternProperties:
"^(ethernet-)?port@[0-6]$":
- type: object
- description: Ethernet switch ports
+ $ref: dsa-port.yaml#

properties:
qca,sgmii-rxclk-falling-edge:
@@ -102,7 +101,7 @@ patternProperties:
SGMII on the QCA8337, it is advised to set this unless a communication
issue is observed.

- unevaluatedProperties: true
+ unevaluatedProperties: false

oneOf:
- required:
diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
index 1a7d45a8ad66..ad1793eba31a 100644
--- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
@@ -7,7 +7,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Realtek switches for unmanaged switches

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base

maintainers:
- Linus Walleij <[email protected]>
diff --git a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
index 7ca9c19a157c..eb9ea25efcb7 100644
--- a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
@@ -14,7 +14,7 @@ description: |
handles 4 ports + 1 CPU management port.

allOf:
- - $ref: dsa.yaml#
+ - $ref: dsa.yaml#/$defs/base

properties:
compatible:
diff --git a/Documentation/devicetree/bindings/net/ethernet-switch.yaml b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
index f698857619da..0d417997c163 100644
--- a/Documentation/devicetree/bindings/net/ethernet-switch.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
@@ -25,6 +25,8 @@ properties:
patternProperties:
"^(ethernet-)?ports$":
type: object
+ additionalProperties: false
+
properties:
'#address-cells':
const: 1
@@ -36,10 +38,6 @@ patternProperties:
type: object
description: Ethernet switch ports

- allOf:
- - $ref: /schemas/net/dsa/dsa-port.yaml#
- - $ref: ethernet-switch-port.yaml#
-
oneOf:
- required:
- ports
@@ -48,4 +46,20 @@ oneOf:

additionalProperties: true

+$defs:
+ base:
+ description: An Ethernet switch without any extra port properties
+ $ref: '#/'
+
+ patternProperties:
+ "^(ethernet-)?ports$":
+ type: object
+
+ patternProperties:
+ "^(ethernet-)?port@[0-9]+$":
+ description: Ethernet switch ports
+ $ref: ethernet-switch-port.yaml#
+ unevaluatedProperties: false
+
+
...

2022-11-05 18:45:28

by Colin Foster

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 3/6] dt-bindings: net: dsa: mediatek,mt7530: remove unnecessary dsa-port reference

Hi Rob,

On Fri, Nov 04, 2022 at 01:53:43PM -0500, Rob Herring wrote:
> On Thu, Nov 03, 2022 at 09:52:01PM -0700, Colin Foster wrote:
> > dsa.yaml contains a reference to dsa-port.yaml, so a duplicate reference to
> > the binding isn't necessary. Remove this unnecessary reference.
> >
> > Signed-off-by: Colin Foster <[email protected]>
> > Suggested-by: Vladimir Oltean <[email protected]>
> > Reviewed-by: Arınç ÜNAL <[email protected]>
> > ---
> >
> > v1 -> v2
> > * Add Reviewed-by
> >
> > ---
> > Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> > index f2e9ff3f580b..81f291105660 100644
> > --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> > +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> > @@ -159,8 +159,6 @@ patternProperties:
> > type: object
> > description: Ethernet switch ports
> >
> > - unevaluatedProperties: false
> > -
>
> You just allowed this node to have any property.

I appreciate your time and help. Thank you. In this case, I think I need
"unevaluatedProperties: true" so that the ^(ethernet-)?port node can
get the properties it needs from nodes in dsa... ?

But then I'm not sure how this node worked in the first place. I might
have misunderstood, but I thought you suggested that if this node had
unevaluatedProperties: false, it wouldn't be able to look into dsa-port.

On the other hand, you did include a lot more information in your
response to 0/6 of this set, which I have yet to fully absorb.

>
> > properties:
> > reg:
> > description:
> > @@ -168,7 +166,6 @@ patternProperties:
> > for user ports.
> >
> > allOf:
> > - - $ref: dsa-port.yaml#
> > - if:
> > required: [ ethernet ]
> > then:
> > --
> > 2.25.1
> >
> >

2022-11-09 23:35:41

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 4/6] dt-bindings: net: add generic ethernet-switch

On Thu, Nov 03, 2022 at 09:52:02PM -0700, Colin Foster wrote:
> diff --git a/Documentation/devicetree/bindings/net/ethernet-switch.yaml b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
> new file mode 100644
> index 000000000000..fbaac536673d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ethernet-switch.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ethernet-switch.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ethernet Switch Device Tree Bindings

I vaguely remember Krzysztof saying during other reviews that "Device
Tree Bindings" in the title is superfluous. Suggest: "Generic Ethernet
Switch".

> +
> +maintainers:
> + - Andrew Lunn <[email protected]>
> + - Florian Fainelli <[email protected]>
> + - Vivien Didelot <[email protected]>
> +
> +description:
> + This binding represents Ethernet Switches which have a dedicated CPU
> + port. That port is usually connected to an Ethernet Controller of the
> + SoC. Such setups are typical for embedded devices.

This description was taken from the DSA switch schema and not adapted
for the generic Ethernet switch schema.

Suggest instead:

Ethernet switches are multi-port Ethernet controllers. Each port has
its own number and is represented as its own Ethernet controller.
The minimum required functionality is to pass packets to software.
They may or may not be able to forward packets autonomously between
ports.

(this should also clarify if it's okay to reference ethernet-switch.yaml
from drivers which don't speak switchdev, which I believe it is).

(my suggestion is open to further comments for improvement)

> +
> +select: false
> +
> +properties:
> + $nodename:
> + pattern: "^(ethernet-)?switch(@.*)?$"
> +
> +patternProperties:
> + "^(ethernet-)?ports$":
> + type: object
> + properties:
> + '#address-cells':
> + const: 1
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + "^(ethernet-)?port@[0-9]+$":
> + type: object
> + description: Ethernet switch ports
> +
> + $ref: /schemas/net/dsa/dsa-port.yaml#

I wonder if you actually meant dsa-port.yaml and not ethernet-controller.yaml?

> +
> +oneOf:
> + - required:
> + - ports
> + - required:
> + - ethernet-ports
> +
> +additionalProperties: true
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3106a9f0567a..3b6c3989c419 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14326,6 +14326,7 @@ M: Florian Fainelli <[email protected]>
> M: Vladimir Oltean <[email protected]>
> S: Maintained
> F: Documentation/devicetree/bindings/net/dsa/
> +F: Documentation/devicetree/bindings/net/ethernet-switch.yaml
> F: drivers/net/dsa/
> F: include/linux/dsa/
> F: include/linux/platform_data/dsa.h
> --
> 2.25.1
>


2022-11-09 23:56:00

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 2/6] dt-bindings: net: dsa: qca8k: utilize shared dsa.yaml

On Thu, Nov 03, 2022 at 09:52:00PM -0700, Colin Foster wrote:
> The dsa.yaml binding contains duplicated bindings for address and size
> cells, as well as the reference to dsa-port.yaml. Instead of duplicating
> this information, remove the reference to dsa-port.yaml and include the
> full reference to dsa.yaml.
>
> Signed-off-by: Colin Foster <[email protected]>
> Suggested-by: Vladimir Oltean <[email protected]>
> ---
>
> v1 -> v2
> * Add #address-cells and #size-cells to the switch layer. They aren't
> part of dsa.yaml.

I'm afraid this is not the correct resolution to the warnings you saw.
There is no reason to have #address-cells = <1> under the "switch" node,
since none of that node's children have a unit address (see: "ports", "mdio").
The schema example is wrong, please fix that.

2022-11-10 00:23:58

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 5/6] dt-bindings: net: add generic ethernet-switch-port binding

On Fri, Nov 04, 2022 at 01:50:28PM -0500, Rob Herring wrote:
> Or we can just allow anything from ethernet-controller.yaml and drop
> this list.

Sounds fine to me. Makes ethernet-switch-port just an ethernet-controller
where "reg" has a particular meaning (port number). Which is basically true.