[PATCH net-next v2 00/10] define and enforce phylink bindings

[PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

Hello there.

This patch series defines phylink bindings and enforces them for the
ethernet controllers that need them.

Some schemas had to be changed to properly enforce phylink bindings for all
of the affected ethernet controllers. Some of the documents of these
ethernet controllers were non json-schema, which had to be converted.

I will convert the remaining documents to json-schema while this patch
series receives reviews.

Cheers.
Arınç

v2:
After many discussions on v1, the concept of the patch series changed a lot
to document the changes from v1.

Link to v1:
https://lore.kernel.org/netdev/20230812091708.34665-1-arinc.unal@arinc9.com/

[PATCH net-next v2 01/10] net: dsa: mt7530: register OF node for internal MDIO bus

[Arınç ÜNAL]

From: David Bauer <mail@david-bauer.net>

The MT753x switches provide a switch-internal MDIO bus for the embedded
PHYs.

Register a OF sub-node on the switch OF-node for this internal MDIO bus.
This allows to configure the embedded PHYs using device-tree.

Signed-off-by: David Bauer <mail@david-bauer.net>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 drivers/net/dsa/mt7530.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 035a34b50f31..7db9af7b9658 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2156,10 +2156,13 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
 {
 	struct dsa_switch *ds = priv->ds;
 	struct device *dev = priv->dev;
+	struct device_node *np, *mnp;
 	struct mii_bus *bus;
 	static int idx;
 	int ret;
 
+	np = priv->dev->of_node;
+
 	bus = devm_mdiobus_alloc(dev);
 	if (!bus)
 		return -ENOMEM;
@@ -2178,7 +2181,9 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
 	if (priv->irq)
 		mt7530_setup_mdio_irq(priv);
 
-	ret = devm_mdiobus_register(dev, bus);
+	mnp = of_get_child_by_name(np, "mdio");
+	ret = devm_of_mdiobus_register(dev, bus, mnp);
+	of_node_put(mnp);
 	if (ret) {
 		dev_err(dev, "failed to register MDIO bus: %d\n", ret);
 		if (priv->irq)
-- 
2.39.2

[PATCH net-next v2 02/10] dt-bindings: net: dsa: microchip,lan937x: add missing ethernet on example

[Arınç ÜNAL]

The port@5 node on the example is supposed to be used as a CPU port. Add
the missing ethernet property and remove the label property. Remove the MAC
bindings on the example as they cannot be validated.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 .../bindings/net/dsa/microchip,lan937x.yaml          | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
index 8d7e878b84dc..d187034fb31a 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
@@ -68,16 +68,6 @@ examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
 
-    macb0 {
-            #address-cells = <1>;
-            #size-cells = <0>;
-
-            fixed-link {
-                    speed = <1000>;
-                    full-duplex;
-            };
-    };
-
     spi {
             #address-cells = <1>;
             #size-cells = <0>;
@@ -134,10 +124,10 @@ examples:
 
                             port@5 {
                                     reg = <5>;
-                                    label = "lan7";
                                     phy-mode = "rgmii";
                                     tx-internal-delay-ps = <2000>;
                                     rx-internal-delay-ps = <2000>;
+                                    ethernet = <&macb1>;
 
                                     fixed-link {
                                             speed = <1000>;
-- 
2.39.2

[PATCH net-next v2 03/10] dt-bindings: net: dsa: realtek: fix MDIO bus child node restrictions

[Arınç ÜNAL]

The MDIO controlled Realtek switches (distinguished by the reg property
being used) are allowed to have their MDIO bus registered OF-based. Allow
the mdio property for them.

Only the SMI controlled Realtek switches require the compatible property of
the mdio child node. Require it for them and disallow it for the MDIO controlled
ones.

Remove the unnecessary if:then:not:required rule and comments.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../devicetree/bindings/net/dsa/realtek.yaml  | 20 ++++++++-----------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
index cfd69c2604ea..7eb025df0df8 100644
--- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
@@ -109,18 +109,19 @@ if:
 
 then:
   $ref: /schemas/spi/spi-peripheral-props.yaml#
-  not:
-    required:
-      - mdc-gpios
-      - mdio-gpios
-      - mdio
-
   properties:
     mdc-gpios: false
     mdio-gpios: false
-    mdio: false
+    mdio:
+      properties:
+        compatible: false
 
 else:
+  properties:
+    mdio:
+      required:
+        - compatible
+
   required:
     - mdc-gpios
     - mdio-gpios
@@ -130,11 +131,6 @@ else:
 required:
   - compatible
 
-    #  - mdc-gpios
-    #  - mdio-gpios
-    #  - reset-gpios
-    #  - mdio
-
 unevaluatedProperties: false
 
 examples:
-- 
2.39.2

[PATCH net-next v2 04/10] dt-bindings: net: dsa: nxp,sja1105: improve MDIO bus bindings

[Arınç ÜNAL]

The SJA1110 switch uses the mdios property for its two MDIO buses. Instead
of a pattern, define two mdio nodes. This ensures the same compatible
string won't be used twice. The address and size cell definitions can also
be removed now that the reg property has become unnecessary.

Move the comment to the description of mdios, mdio0, and mdio1 properties.
Disallow the mdios property for SJA1105. Require at least one of the MDIO
buses to be defined to prevent empty mdios child node.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../bindings/net/dsa/nxp,sja1105.yaml         | 49 ++++++++++---------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
index 4d5f5cc6d031..3f3c4ecc6442 100644
--- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
@@ -35,42 +35,44 @@ properties:
   spi-cpha: true
   spi-cpol: true
 
-  # Optional container node for the 2 internal MDIO buses of the SJA1110
-  # (one for the internal 100base-T1 PHYs and the other for the single
-  # 100base-TX PHY). The "reg" property does not have physical significance.
-  # The PHY addresses to port correspondence is as follows: for 100base-T1,
-  # port 5 has PHY 1, port 6 has PHY 2 etc, while for 100base-TX, port 1 has
-  # PHY 1.
   mdios:
+    description:
+      The optional container node for the two MDIO buses of the SJA1110.
     type: object
 
     properties:
-      '#address-cells':
-        const: 1
-      '#size-cells':
-        const: 0
-
-    patternProperties:
-      "^mdio@[0-1]$":
+      mdio0:
+        description:
+          The node for the bus controlling the 100base-T1 PHYs of the SJA1110
+          switch. PHY address to port correspondence is port 5 has PHY 1, port 6
+          has PHY 2, etc.
         $ref: /schemas/net/mdio.yaml#
         unevaluatedProperties: false
 
         properties:
           compatible:
-            oneOf:
-              - enum:
-                  - nxp,sja1110-base-t1-mdio
-                  - nxp,sja1110-base-tx-mdio
+            const: nxp,sja1110-base-t1-mdio
+
+        required:
+          - compatible
 
-          reg:
-            oneOf:
-              - enum:
-                  - 0
-                  - 1
+      mdio1:
+        description:
+          The node for the bus controlling the 100base-TX PHY of the SJA1110
+          switch. PHY address to port correspondence is port 1 has PHY 1.
+        $ref: /schemas/net/mdio.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          compatible:
+            const: nxp,sja1110-base-tx-mdio
 
         required:
           - compatible
-          - reg
+
+    anyOf:
+      - required: [ mdio0 ]
+      - required: [ mdio1 ]
 
 patternProperties:
   "^(ethernet-)?ports$":
@@ -124,6 +126,7 @@ allOf:
     then:
       properties:
         spi-cpol: false
+        mdios: false
       required:
         - spi-cpha
     else:
-- 
2.39.2

Re: [PATCH net-next v2 04/10] dt-bindings: net: dsa: nxp,sja1105: improve MDIO bus bindings

[Rob Herring]

On Sat, Sep 16, 2023 at 02:08:56PM +0300, Arınç ÜNAL wrote:
> The SJA1110 switch uses the mdios property for its two MDIO buses. Instead
> of a pattern, define two mdio nodes. This ensures the same compatible
> string won't be used twice. The address and size cell definitions can also
> be removed now that the reg property has become unnecessary.
> 
> Move the comment to the description of mdios, mdio0, and mdio1 properties.
> Disallow the mdios property for SJA1105. Require at least one of the MDIO
> buses to be defined to prevent empty mdios child node.

It's an ABI. You can't just change this.

You can split the pattern into 'mdio@0' and 'mdio@1' with different 
compatibles for each if you want.

Rob

[PATCH net-next v2 05/10] dt-bindings: net: dsa: define MDIO bus child node

[Arınç ÜNAL]

Some DSA subdrivers register the MDIO bus of the switch they control. Or
let the DSA driver register the MDIO bus. The node for these buses are
either required or optional, depending on the subdriver. Document this on
all of the affected DSA switch schemas.

The attributes of a DSA subdriver that lets the DSA driver register the
bus:
- ds->ops->phy_read() and ds->ops->phy_write() are present.
- ds->slave_mii_bus is not populated by the DSA subdriver.
- The bus is registered non-OF-based or OF-based. Registered OF-based if
  "mdio" child node is defined.

The affected DSA switch schemas are documented below.

- brcm,b53.yaml

drivers/net/dsa/b53/b53_common.c:
- The DSA subdriver lets the DSA driver register the bus.

---

- mediatek,mt7530.yaml

drivers/net/dsa/mt7530.c:
- The DSA subdriver won't let the DSA driver register the bus.
  - No ds->ops->phy_read() or ds->ops->phy_write().
- Registers the bus non-OF-based or OF-based. Registers OF-based if "mdio"
  child node is defined.

---

- microchip,ksz.yaml
- microchip,lan937x.yaml

drivers/net/dsa/microchip/ksz_common.c:
- The DSA subdriver won't let the DSA driver register the bus when "mdio"
  child node is defined. Registers the bus OF-based only. Registers the bus
  when "mdio" child node is defined.
  - mdio_np = of_get_child_by_name(dev->dev->of_node, "mdio");
    if (!mdio_np)
      return 0;
    ds->slave_mii_bus = bus;

---

- nxp,sja1105.yaml

drivers/net/dsa/sja1105/sja1105_mdio.c:
- The DSA subdriver won't let the DSA driver register the bus.
  - No ds->ops->phy_read() or ds->ops->phy_write().
- Registers multiple buses OF-based only. Registers the buses when "mdios"
  child node and "nxp,sja1110-base-tx-mdio" and "nxp,sja1110-base-t1-mdio"
  compatible strings per child node under "mdios" is defined.
  - mdio_node = of_get_child_by_name(switch_node, "mdios");
    if (!mdio_node)
      return 0;
  - np = of_get_compatible_child(mdio_node, "nxp,sja1110-base-tx-mdio");
    if (!np)
      return 0;
  - np = of_get_compatible_child(mdio_node, "nxp,sja1110-base-t1-mdio");
    if (!np)
      return 0;

---

- qca8k.yaml

drivers/net/dsa/qca/qca8k-8xxx.c:
- The DSA subdriver won't let the DSA driver register the bus.
  - No ds->ops->phy_read() or ds->ops->phy_write().
- Registers the bus non-OF-based or OF-based. Registers OF-based if "mdio"
  child node is defined.
  - mdio = of_get_child_by_name(priv->dev->of_node, "mdio");

---

- realtek.yaml

drivers/net/dsa/realtek/realtek-smi.c:
- The DSA subdriver won't let the DSA driver register the bus. Registers
  the bus OF-based only. Registering the bus is mandatory. Registers the
  bus when compatible string "realtek,smi-mdio" on a child node is defined.
  - mdio_np = of_get_compatible_child(priv->dev->of_node, "realtek,smi-mdio");
    if (!mdio_np)
      return -ENODEV;
    ds->slave_mii_bus = priv->slave_mii_bus;

drivers/net/dsa/realtek/realtek-mdio.c:
- The DSA subdriver lets the DSA driver register the bus.

---

- renesas,rzn1-a5psw.yaml

drivers/net/dsa/rzn1_a5psw.c:
- The DSA subdriver won't let the DSA driver register the bus.
  - No ds->ops->phy_read() or ds->ops->phy_write().
- Registers the bus OF-based only. Registers the bus when "mdio" child node
  is defined.
  -	mdio = of_get_child_by_name(dev->of_node, "mdio");
    if (of_device_is_available(mdio))
      ret = a5psw_probe_mdio(a5psw, mdio);

---

- ar9331.txt

drivers/net/dsa/qca/ar9331.c:
- The DSA subdriver won't let the DSA driver register the bus.
  - No ds->ops->phy_read() or ds->ops->phy_write().
- Registers the bus OF-based only. Registering the bus is mandatory.
  Registers the bus when "mdio" child node is defined.
  - mnp = of_get_child_by_name(np, "mdio");
    if (!mnp)
      return -ENODEV;

---

- lan9303.txt

drivers/net/dsa/lan9303-core.c:
- The DSA subdriver lets the DSA driver register the bus.

---

- lantiq-gswip.txt

drivers/net/dsa/lantiq_gswip.c:
- The DSA subdriver won't let the DSA driver register the bus.
  - No ds->ops->phy_read() or ds->ops->phy_write().
- Registers the bus OF-based only. Registers the bus when compatible string
  "lantiq,xrx200-mdio" on a child node is defined.
  - mdio_np = of_get_compatible_child(dev->of_node, "lantiq,xrx200-mdio");
    if (mdio_np)
      err = gswip_mdio(priv, mdio_np);

---

- marvell.txt

drivers/net/dsa/mv88e6xxx/chip.c:
- The DSA subdriver won't let the DSA driver register the bus.
  - ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
- Registers the bus non-OF-based or OF-based. Registers OF-based if "mdio"
  child node is defined.

---

- vitesse,vsc73xx.txt

drivers/net/dsa/vitesse-vsc73xx-core.c:
- The DSA subdriver lets the DSA driver register the bus.

I will convert the non json-schema documents accordingly.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../devicetree/bindings/net/dsa/brcm,b53.yaml          |  7 +++++++
 .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml   |  7 +++++++
 .../devicetree/bindings/net/dsa/microchip,ksz.yaml     |  7 +++++++
 .../devicetree/bindings/net/dsa/microchip,lan937x.yaml |  3 +++
 .../devicetree/bindings/net/dsa/nxp,sja1105.yaml       |  3 ++-
 Documentation/devicetree/bindings/net/dsa/qca8k.yaml   | 10 +++++-----
 Documentation/devicetree/bindings/net/dsa/realtek.yaml |  4 ++++
 .../bindings/net/dsa/renesas,rzn1-a5psw.yaml           |  3 +++
 8 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
index 4c78c546343f..0bb2ff0cf2d0 100644
--- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
@@ -65,6 +65,13 @@ properties:
               - brcm,bcm63268-switch
           - const: brcm,bcm63xx-switch
 
+  mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus will be
+      registered non-OF-based if this is not defined.
+    $ref: /schemas/net/mdio.yaml#
+    unevaluatedProperties: false
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index e532c6b795f4..31a7dbbf704d 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -151,6 +151,13 @@ properties:
       ethsys.
     maxItems: 1
 
+  mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus will be
+      registered non-OF-based if this is not defined.
+    $ref: /schemas/net/mdio.yaml#
+    unevaluatedProperties: false
+
 patternProperties:
   "^(ethernet-)?ports$":
     type: object
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
index e51be1ac0362..20e4174fe1ab 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
@@ -49,6 +49,13 @@ properties:
       Set if the output SYNCLKO clock should be disabled. Do not mix with
       microchip,synclko-125.
 
+  mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus will be
+      registered non-OF-based if this is not defined.
+    $ref: /schemas/net/mdio.yaml#
+    unevaluatedProperties: false
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
index d187034fb31a..33a4926b2d94 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
@@ -32,6 +32,9 @@ properties:
     maxItems: 1
 
   mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus will be
+      registered non-OF-based if this is not defined.
     $ref: /schemas/net/mdio.yaml#
     unevaluatedProperties: false
 
diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
index 3f3c4ecc6442..6838dc165d06 100644
--- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
@@ -37,7 +37,8 @@ properties:
 
   mdios:
     description:
-      The optional container node for the two MDIO buses of the SJA1110.
+      The optional container node for the two MDIO buses of the SJA1110. The bus
+      won't be registered if its node is not defined.
     type: object
 
     properties:
diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index df64eebebe18..c472050582be 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -60,13 +60,13 @@ properties:
       B68 on the QCA832x and B49 on the QCA833x.
 
   mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus will be
+      registered non-OF-based if this is not defined. In that case, the legacy
+      mapping will be used. With the legacy mapping, the reg corresponding to
+      the MDIO bus is the switch reg with an offset of -1.
     $ref: /schemas/net/mdio.yaml#
     unevaluatedProperties: false
-    description: Qca8k switch have an internal mdio to access switch port.
-                 If this is not present, the legacy mapping is used and the
-                 internal mdio access is used.
-                 With the legacy mapping the reg corresponding to the internal
-                 mdio is the switch reg with an offset of -1.
 
 $ref: dsa.yaml#
 
diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
index 7eb025df0df8..62ebaa4b5ae3 100644
--- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
@@ -96,6 +96,10 @@ properties:
       - '#interrupt-cells'
 
   mdio:
+    description:
+      The node for the MDIO bus of the switch. For the MDIO controlled switches,
+      this is optional and the bus will be registered non-OF-based if this is
+      not defined.
     $ref: /schemas/net/mdio.yaml#
     unevaluatedProperties: false
 
diff --git a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
index 833d2f68daa1..9ad9f5cdf688 100644
--- a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
@@ -46,6 +46,9 @@ properties:
     maxItems: 1
 
   mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus won't be
+      registered if this is not defined.
     $ref: /schemas/net/mdio.yaml#
     unevaluatedProperties: false
 
-- 
2.39.2

Re: [PATCH net-next v2 05/10] dt-bindings: net: dsa: define MDIO bus child node

[Andrew Lunn]

On Sat, Sep 16, 2023 at 02:08:57PM +0300, Arınç ÜNAL wrote:
> Some DSA subdrivers register the MDIO bus of the switch they control. Or
> let the DSA driver register the MDIO bus. The node for these buses are
> either required or optional, depending on the subdriver. Document this on
> all of the affected DSA switch schemas.
> 
> The attributes of a DSA subdriver that lets the DSA driver register the
> bus:
> - ds->ops->phy_read() and ds->ops->phy_write() are present.
> - ds->slave_mii_bus is not populated by the DSA subdriver.
> - The bus is registered non-OF-based or OF-based. Registered OF-based if
>   "mdio" child node is defined.
> 
> The affected DSA switch schemas are documented below.
> 
> - brcm,b53.yaml
> 
> drivers/net/dsa/b53/b53_common.c:
> - The DSA subdriver lets the DSA driver register the bus.
> 
> ---

git uses --- to separate the commit message from additional comments
for the reviewers. Anything after the --- will not be merged. Is that
your intention?

     Andrew

Re: [PATCH net-next v2 05/10] dt-bindings: net: dsa: define MDIO bus child node

[Arınç ÜNAL]

On 16.09.2023 17:11, Andrew Lunn wrote:
> On Sat, Sep 16, 2023 at 02:08:57PM +0300, Arınç ÜNAL wrote:
>> Some DSA subdrivers register the MDIO bus of the switch they control. Or
>> let the DSA driver register the MDIO bus. The node for these buses are
>> either required or optional, depending on the subdriver. Document this on
>> all of the affected DSA switch schemas.
>>
>> The attributes of a DSA subdriver that lets the DSA driver register the
>> bus:
>> - ds->ops->phy_read() and ds->ops->phy_write() are present.
>> - ds->slave_mii_bus is not populated by the DSA subdriver.
>> - The bus is registered non-OF-based or OF-based. Registered OF-based if
>>    "mdio" child node is defined.
>>
>> The affected DSA switch schemas are documented below.
>>
>> - brcm,b53.yaml
>>
>> drivers/net/dsa/b53/b53_common.c:
>> - The DSA subdriver lets the DSA driver register the bus.
>>
>> ---
> 
> git uses --- to separate the commit message from additional comments
> for the reviewers. Anything after the --- will not be merged. Is that
> your intention?

No, they're vital information. I'll find something better to separate them,
thanks.

Arınç

Re: [PATCH net-next v2 05/10] dt-bindings: net: dsa: define MDIO bus child node

[Rob Herring]

On Sat, Sep 16, 2023 at 02:08:57PM +0300, Arınç ÜNAL wrote:
> Some DSA subdrivers register the MDIO bus of the switch they control. Or
> let the DSA driver register the MDIO bus. The node for these buses are
> either required or optional, depending on the subdriver. Document this on
> all of the affected DSA switch schemas.
> 
> The attributes of a DSA subdriver that lets the DSA driver register the
> bus:
> - ds->ops->phy_read() and ds->ops->phy_write() are present.
> - ds->slave_mii_bus is not populated by the DSA subdriver.
> - The bus is registered non-OF-based or OF-based. Registered OF-based if
>   "mdio" child node is defined.
> 
> The affected DSA switch schemas are documented below.
> 
> - brcm,b53.yaml
> 
> drivers/net/dsa/b53/b53_common.c:
> - The DSA subdriver lets the DSA driver register the bus.
> 
> ---

This will cause everything after here to be dropped when the patch is 
applied.

In any case, write binding commit message without Linux details. 
Ideally, variations should be based on hardware differences not driver 
policies.

> 
> - mediatek,mt7530.yaml
> 
> drivers/net/dsa/mt7530.c:
> - The DSA subdriver won't let the DSA driver register the bus.
>   - No ds->ops->phy_read() or ds->ops->phy_write().
> - Registers the bus non-OF-based or OF-based. Registers OF-based if "mdio"
>   child node is defined.
> 
> ---
> 
> - microchip,ksz.yaml
> - microchip,lan937x.yaml
> 
> drivers/net/dsa/microchip/ksz_common.c:
> - The DSA subdriver won't let the DSA driver register the bus when "mdio"
>   child node is defined. Registers the bus OF-based only. Registers the bus
>   when "mdio" child node is defined.
>   - mdio_np = of_get_child_by_name(dev->dev->of_node, "mdio");
>     if (!mdio_np)
>       return 0;
>     ds->slave_mii_bus = bus;
> 
> ---
> 
> - nxp,sja1105.yaml
> 
> drivers/net/dsa/sja1105/sja1105_mdio.c:
> - The DSA subdriver won't let the DSA driver register the bus.
>   - No ds->ops->phy_read() or ds->ops->phy_write().
> - Registers multiple buses OF-based only. Registers the buses when "mdios"
>   child node and "nxp,sja1110-base-tx-mdio" and "nxp,sja1110-base-t1-mdio"
>   compatible strings per child node under "mdios" is defined.
>   - mdio_node = of_get_child_by_name(switch_node, "mdios");
>     if (!mdio_node)
>       return 0;
>   - np = of_get_compatible_child(mdio_node, "nxp,sja1110-base-tx-mdio");
>     if (!np)
>       return 0;
>   - np = of_get_compatible_child(mdio_node, "nxp,sja1110-base-t1-mdio");
>     if (!np)
>       return 0;
> 
> ---
> 
> - qca8k.yaml
> 
> drivers/net/dsa/qca/qca8k-8xxx.c:
> - The DSA subdriver won't let the DSA driver register the bus.
>   - No ds->ops->phy_read() or ds->ops->phy_write().
> - Registers the bus non-OF-based or OF-based. Registers OF-based if "mdio"
>   child node is defined.
>   - mdio = of_get_child_by_name(priv->dev->of_node, "mdio");
> 
> ---
> 
> - realtek.yaml
> 
> drivers/net/dsa/realtek/realtek-smi.c:
> - The DSA subdriver won't let the DSA driver register the bus. Registers
>   the bus OF-based only. Registering the bus is mandatory. Registers the
>   bus when compatible string "realtek,smi-mdio" on a child node is defined.
>   - mdio_np = of_get_compatible_child(priv->dev->of_node, "realtek,smi-mdio");
>     if (!mdio_np)
>       return -ENODEV;
>     ds->slave_mii_bus = priv->slave_mii_bus;
> 
> drivers/net/dsa/realtek/realtek-mdio.c:
> - The DSA subdriver lets the DSA driver register the bus.
> 
> ---
> 
> - renesas,rzn1-a5psw.yaml
> 
> drivers/net/dsa/rzn1_a5psw.c:
> - The DSA subdriver won't let the DSA driver register the bus.
>   - No ds->ops->phy_read() or ds->ops->phy_write().
> - Registers the bus OF-based only. Registers the bus when "mdio" child node
>   is defined.
>   -	mdio = of_get_child_by_name(dev->of_node, "mdio");
>     if (of_device_is_available(mdio))
>       ret = a5psw_probe_mdio(a5psw, mdio);
> 
> ---
> 
> - ar9331.txt
> 
> drivers/net/dsa/qca/ar9331.c:
> - The DSA subdriver won't let the DSA driver register the bus.
>   - No ds->ops->phy_read() or ds->ops->phy_write().
> - Registers the bus OF-based only. Registering the bus is mandatory.
>   Registers the bus when "mdio" child node is defined.
>   - mnp = of_get_child_by_name(np, "mdio");
>     if (!mnp)
>       return -ENODEV;
> 
> ---
> 
> - lan9303.txt
> 
> drivers/net/dsa/lan9303-core.c:
> - The DSA subdriver lets the DSA driver register the bus.
> 
> ---
> 
> - lantiq-gswip.txt
> 
> drivers/net/dsa/lantiq_gswip.c:
> - The DSA subdriver won't let the DSA driver register the bus.
>   - No ds->ops->phy_read() or ds->ops->phy_write().
> - Registers the bus OF-based only. Registers the bus when compatible string
>   "lantiq,xrx200-mdio" on a child node is defined.
>   - mdio_np = of_get_compatible_child(dev->of_node, "lantiq,xrx200-mdio");
>     if (mdio_np)
>       err = gswip_mdio(priv, mdio_np);
> 
> ---
> 
> - marvell.txt
> 
> drivers/net/dsa/mv88e6xxx/chip.c:
> - The DSA subdriver won't let the DSA driver register the bus.
>   - ds->slave_mii_bus = mv88e6xxx_default_mdio_bus(chip);
> - Registers the bus non-OF-based or OF-based. Registers OF-based if "mdio"
>   child node is defined.
> 
> ---
> 
> - vitesse,vsc73xx.txt
> 
> drivers/net/dsa/vitesse-vsc73xx-core.c:
> - The DSA subdriver lets the DSA driver register the bus.
> 
> I will convert the non json-schema documents accordingly.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../devicetree/bindings/net/dsa/brcm,b53.yaml          |  7 +++++++
>  .../devicetree/bindings/net/dsa/mediatek,mt7530.yaml   |  7 +++++++
>  .../devicetree/bindings/net/dsa/microchip,ksz.yaml     |  7 +++++++
>  .../devicetree/bindings/net/dsa/microchip,lan937x.yaml |  3 +++
>  .../devicetree/bindings/net/dsa/nxp,sja1105.yaml       |  3 ++-
>  Documentation/devicetree/bindings/net/dsa/qca8k.yaml   | 10 +++++-----
>  Documentation/devicetree/bindings/net/dsa/realtek.yaml |  4 ++++
>  .../bindings/net/dsa/renesas,rzn1-a5psw.yaml           |  3 +++
>  8 files changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> index 4c78c546343f..0bb2ff0cf2d0 100644
> --- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> @@ -65,6 +65,13 @@ properties:
>                - brcm,bcm63268-switch
>            - const: brcm,bcm63xx-switch
>  
> +  mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus will be
> +      registered non-OF-based if this is not defined.
> +    $ref: /schemas/net/mdio.yaml#
> +    unevaluatedProperties: false
> +
>  required:
>    - compatible
>    - reg
> diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> index e532c6b795f4..31a7dbbf704d 100644
> --- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
> @@ -151,6 +151,13 @@ properties:
>        ethsys.
>      maxItems: 1
>  
> +  mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus will be
> +      registered non-OF-based if this is not defined.
> +    $ref: /schemas/net/mdio.yaml#
> +    unevaluatedProperties: false
> +
>  patternProperties:
>    "^(ethernet-)?ports$":
>      type: object
> diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> index e51be1ac0362..20e4174fe1ab 100644
> --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> @@ -49,6 +49,13 @@ properties:
>        Set if the output SYNCLKO clock should be disabled. Do not mix with
>        microchip,synclko-125.
>  
> +  mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus will be
> +      registered non-OF-based if this is not defined.
> +    $ref: /schemas/net/mdio.yaml#
> +    unevaluatedProperties: false
> +
>  required:
>    - compatible
>    - reg
> diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
> index d187034fb31a..33a4926b2d94 100644
> --- a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
> @@ -32,6 +32,9 @@ properties:
>      maxItems: 1
>  
>    mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus will be
> +      registered non-OF-based if this is not defined.
>      $ref: /schemas/net/mdio.yaml#
>      unevaluatedProperties: false
>  
> diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> index 3f3c4ecc6442..6838dc165d06 100644
> --- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
> @@ -37,7 +37,8 @@ properties:
>  
>    mdios:
>      description:
> -      The optional container node for the two MDIO buses of the SJA1110.
> +      The optional container node for the two MDIO buses of the SJA1110. The bus
> +      won't be registered if its node is not defined.
>      type: object
>  
>      properties:
> diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
> index df64eebebe18..c472050582be 100644
> --- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
> @@ -60,13 +60,13 @@ properties:
>        B68 on the QCA832x and B49 on the QCA833x.
>  
>    mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus will be
> +      registered non-OF-based if this is not defined. In that case, the legacy
> +      mapping will be used. With the legacy mapping, the reg corresponding to
> +      the MDIO bus is the switch reg with an offset of -1.
>      $ref: /schemas/net/mdio.yaml#
>      unevaluatedProperties: false
> -    description: Qca8k switch have an internal mdio to access switch port.
> -                 If this is not present, the legacy mapping is used and the
> -                 internal mdio access is used.
> -                 With the legacy mapping the reg corresponding to the internal
> -                 mdio is the switch reg with an offset of -1.
>  
>  $ref: dsa.yaml#
>  
> diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> index 7eb025df0df8..62ebaa4b5ae3 100644
> --- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
> @@ -96,6 +96,10 @@ properties:
>        - '#interrupt-cells'
>  
>    mdio:
> +    description:
> +      The node for the MDIO bus of the switch. For the MDIO controlled switches,
> +      this is optional and the bus will be registered non-OF-based if this is
> +      not defined.
>      $ref: /schemas/net/mdio.yaml#
>      unevaluatedProperties: false
>  
> diff --git a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
> index 833d2f68daa1..9ad9f5cdf688 100644
> --- a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
> @@ -46,6 +46,9 @@ properties:
>      maxItems: 1
>  
>    mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus won't be
> +      registered if this is not defined.
>      $ref: /schemas/net/mdio.yaml#
>      unevaluatedProperties: false
>  
> -- 
> 2.39.2
> 

[PATCH net-next v2 06/10] dt-bindings: net: define phylink bindings

[Arınç ÜNAL]

Define the phylink bindings on ethernet-controller.yaml and replace the
bindings on the dsa-port.yaml schema with them. The shared ports of DSA
requires phy-mode so keep that.

Important remarks about the phylink bindings:
- fsl,fman-dtsec uses tbi-handle instead of pcs-handle, therefore
  tbi-handle is included.
- For sfp, only managed with the in-band-status value must be required, but
  phy-handle is required as another option because of a special case on the
  eth2 node on arch/arm/boot/dts/marvell/armada-385-turris-omnia.dts.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../devicetree/bindings/net/dsa/dsa-port.yaml | 12 ++------
 .../bindings/net/ethernet-controller.yaml     | 29 +++++++++++++++++++
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index 480120469953..53efb686cfc7 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -65,16 +65,8 @@ if:
     - required: [ ethernet ]
     - required: [ link ]
 then:
-  allOf:
-    - required:
-        - phy-mode
-    - oneOf:
-        - required:
-            - fixed-link
-        - required:
-            - phy-handle
-        - required:
-            - managed
+  $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+  required: [ phy-mode ]
 
 additionalProperties: true
 
diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 9f6a5ccbcefe..ef28ebc76798 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -284,6 +284,35 @@ allOf:
             controllers that have configurable TX internal delays. If this
             property is present then the MAC applies the TX delay.
 
+$defs:
+  phylink:
+    description: phylink bindings for ethernet controllers
+    allOf:
+      - anyOf:
+          - required: [ fixed-link ]
+          - required: [ pcs-handle ]
+          - required: [ tbi-handle ]
+          - required: [ phy-handle ]
+          - required: [ sfp ]
+          - required: [ managed ]
+
+      - if:
+          required: [ fixed-link ]
+        then:
+          properties:
+            managed:
+              const: auto
+
+      - if:
+          required: [ sfp ]
+        then:
+          oneOf:
+            - required: [ phy-handle ]
+            - required: [ managed ]
+              properties:
+                managed:
+                  const: in-band-status
+
 additionalProperties: true
 
 ...
-- 
2.39.2

[PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Arınç ÜNAL]

Phylink bindings are required for ethernet controllers that utilise
phylink_fwnode_phy_connect() directly or through phylink_of_phy_connect(),
and register OF-based only MDIO buses, if they register any.

All the drivers that utilise phylink_fwnode_phy_connect() directly or
through phylink_of_phy_connect():

- DSA
- drivers/net/ethernet/mscc/ocelot_net.c
  - mscc,vsc7514-switch.yaml
- drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
  - microchip,sparx5-switch.yaml
- drivers/net/ethernet/altera/altera_tse_main.c
  - altr,tse.yaml
- drivers/net/ethernet/xilinx/xilinx_axienet_main.c
  - xlnx,axi-ethernet.yaml
- drivers/net/ethernet/mediatek/mtk_eth_soc.c
  - mediatek,net.yaml
- drivers/net/ethernet/ti/am65-cpsw-nuss.c
  - ti,k3-am654-cpsw-nuss.yaml
- drivers/net/ethernet/atheros/ag71xx.c
  - qca,ar71xx.yaml
- drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
  - fsl,fman-dtsec.yaml
- drivers/net/ethernet/microchip/lan966x/lan966x_main.c
  - microchip,lan966x-switch.yaml
- drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
  - marvell,pp2.yaml
- drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
  - fsl,qoriq-mc-dpmac.yaml
- drivers/net/ethernet/cadence/macb_main.c
  - cdns,macb.yaml
  - Can register non-OF-based bus.
- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
  - snps,dwmac.yaml
  - Can register non-OF-based bus.
- drivers/net/ethernet/marvell/mvneta.c
  - marvell-armada-370-neta.txt
- drivers/net/ethernet/freescale/enetc/enetc.c
  - fsl-enetc.txt

RFC: The drivers marked with "can register non-OF-based bus" seem to search
the MDIO bus to connect the PHY to the MAC using phylink_connect_phy()
and/or phy_find_first() if phylink bindings don't exist. Should we enforce
phylink bindings on their schemas regardless?

DSA, like any other driver utilising the Linux MDIO infrastructure,
can register a bus. On certain conditions, it registers the MDIO
bus of the switch it controls non-OF-based.

We can know when DSA won't create any non-OF-based buses. That leaves us
with only OF-based buses in which case we can enforce phylink bindings for
user ports. The bindings are already enforced for shared ports so we can
enforce the bindings for all ports on the switch schemas which will achieve
the same result with fewer lines.

By looking at whether the mdio child node exists and what is explained on
it, we can enforce phylink bindings.

- mscc,vsc7514-switch.yaml
  - Enforce phylink bindings for all ports.

- arrow,xrs700x.yaml
  - Enforce phylink bindings for all ports.

- brcm,b53.yaml
  - Enforce phylink bindings for all ports if "mdio" is defined.

- brcm,sf2.yaml
  - Enforce phylink bindings for all ports.

- hirschmann,hellcreek.yaml
  - Enforce phylink bindings for all ports.

- mediatek,mt7530.yaml
  - Enforce phylink bindings for all ports if "mdio" is defined.

- microchip,ksz.yaml
  - Enforce phylink bindings for all ports if "mdio" is defined.

- microchip,lan937x.yaml
  - Enforce phylink bindings for all ports if "mdio" is defined.

- mscc,ocelot.yaml
  - Enforce phylink bindings for all ports.

- nxp,sja1105.yaml
  - Enforce phylink bindings for all ports.

- qca8k.yaml
  - Enforce phylink bindings for all ports if "mdio" is defined.

- realtek.yaml
  - Enforce phylink bindings for all ports if "mdio" is defined.

- renesas,rzn1-a5psw.yaml
  - Enforce phylink bindings for all ports.

- ar9331.txt
  - Enforce phylink bindings for all ports.

- lan9303.txt
  - Enforce phylink bindings for all ports if "mdio" is defined.

- lantiq-gswip.txt
  - Enforce phylink bindings for all ports.

- marvell.txt
  - Enforce phylink bindings for all ports if "mdio" is defined.

- vitesse,vsc73xx.txt
  - Enforce phylink bindings for all ports if "mdio" is defined.

I will convert the non json-schema documents accordingly.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../devicetree/bindings/net/altr,tse.yaml     |  1 +
 .../devicetree/bindings/net/cdns,macb.yaml    |  1 +
 .../bindings/net/dsa/arrow,xrs700x.yaml       |  6 ++
 .../devicetree/bindings/net/dsa/brcm,b53.yaml |  9 +++
 .../devicetree/bindings/net/dsa/brcm,sf2.yaml |  6 +-
 .../net/dsa/hirschmann,hellcreek.yaml         |  6 ++
 .../bindings/net/dsa/mediatek,mt7530.yaml     |  9 +++
 .../bindings/net/dsa/microchip,ksz.yaml       |  9 +++
 .../bindings/net/dsa/microchip,lan937x.yaml   |  9 +++
 .../bindings/net/dsa/mscc,ocelot.yaml         |  6 ++
 .../bindings/net/dsa/nxp,sja1105.yaml         |  1 +
 .../devicetree/bindings/net/dsa/qca8k.yaml    |  9 +++
 .../devicetree/bindings/net/dsa/realtek.yaml  | 57 +++++++++++--------
 .../bindings/net/dsa/renesas,rzn1-a5psw.yaml  |  2 +-
 .../bindings/net/fsl,fman-dtsec.yaml          |  1 +
 .../bindings/net/fsl,qoriq-mc-dpmac.yaml      |  1 +
 .../devicetree/bindings/net/marvell,pp2.yaml  |  4 +-
 .../devicetree/bindings/net/mediatek,net.yaml |  1 +
 .../net/microchip,lan966x-switch.yaml         |  4 +-
 .../bindings/net/microchip,sparx5-switch.yaml |  5 +-
 .../bindings/net/mscc,vsc7514-switch.yaml     |  5 ++
 .../devicetree/bindings/net/qca,ar71xx.yaml   |  1 +
 .../devicetree/bindings/net/snps,dwmac.yaml   |  1 +
 .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   |  4 +-
 .../bindings/net/xlnx,axi-ethernet.yaml       |  3 +-
 25 files changed, 130 insertions(+), 31 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/altr,tse.yaml b/Documentation/devicetree/bindings/net/altr,tse.yaml
index f5d3b70af07a..24e0f1213109 100644
--- a/Documentation/devicetree/bindings/net/altr,tse.yaml
+++ b/Documentation/devicetree/bindings/net/altr,tse.yaml
@@ -67,6 +67,7 @@ required:
 
 allOf:
   - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
   - if:
       properties:
         compatible:
diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index bf8894a0257e..a455de04c5f1 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -156,6 +156,7 @@ required:
 
 allOf:
   - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
 
   - if:
       not:
diff --git a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml
index 9565a7402146..7829fc9e202d 100644
--- a/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/arrow,xrs700x.yaml
@@ -29,6 +29,12 @@ properties:
   reg:
     maxItems: 1
 
+patternProperties:
+  "^(ethernet-)?ports$":
+    patternProperties:
+      "^(ethernet-)?port@[0-9]+$":
+        $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
index 0bb2ff0cf2d0..ed5b3eab1948 100644
--- a/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
@@ -142,6 +142,15 @@ allOf:
         reg:
           maxItems: 1
 
+  - if:
+      required: [ mdio ]
+    then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          patternProperties:
+            "^(ethernet-)?port@[0-9]+$":
+              $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 unevaluatedProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml b/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml
index c745407f2f68..6cffcd35a943 100644
--- a/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/brcm,sf2.yaml
@@ -81,7 +81,10 @@ properties:
 
     patternProperties:
       '^port@[0-9a-f]$':
-        $ref: dsa-port.yaml#
+        type: object
+        allOf:
+          - $ref: dsa-port.yaml#
+          - $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
         unevaluatedProperties: false
 
         properties:
@@ -161,6 +164,7 @@ examples:
                     port@0 {
                             label = "gphy";
                             reg = <0>;
+                            phy-handle = <&phy8>;
                     };
             };
     };
diff --git a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml
index 4021b054f684..30b4c7b1fd08 100644
--- a/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml
@@ -67,6 +67,12 @@ properties:
 
     additionalProperties: false
 
+patternProperties:
+  "^(ethernet-)?ports$":
+    patternProperties:
+      "^(ethernet-)?port@[0-9]+$":
+        $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
index 31a7dbbf704d..a8e1134707e4 100644
--- a/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mediatek,mt7530.yaml
@@ -297,6 +297,15 @@ allOf:
         mediatek,mcm: false
         reset-names: false
 
+  - if:
+      required: [ mdio ]
+    then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          patternProperties:
+            "^(ethernet-)?port@[0-9]+$":
+              $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 unevaluatedProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
index 20e4174fe1ab..d8f12234eec6 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
@@ -60,6 +60,15 @@ required:
   - compatible
   - reg
 
+if:
+  required: [ mdio ]
+then:
+  patternProperties:
+    "^(ethernet-)?ports$":
+      patternProperties:
+        "^(ethernet-)?port@[0-9]+$":
+          $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 unevaluatedProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
index 33a4926b2d94..44a892b93eca 100644
--- a/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan937x.yaml
@@ -65,6 +65,15 @@ required:
   - compatible
   - reg
 
+if:
+  required: [ mdio ]
+then:
+  patternProperties:
+    "^(ethernet-)?ports$":
+      patternProperties:
+        "^(ethernet-)?port@[0-9]+$":
+          $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 unevaluatedProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml b/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml
index fe02d05196e4..80cc9c49282b 100644
--- a/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/mscc,ocelot.yaml
@@ -73,6 +73,12 @@ properties:
   little-endian: true
   big-endian: true
 
+patternProperties:
+  "^(ethernet-)?ports$":
+    patternProperties:
+      "^(ethernet-)?port@[0-9]+$":
+        $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
index 6838dc165d06..1d5c4a81caf6 100644
--- a/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/nxp,sja1105.yaml
@@ -79,6 +79,7 @@ patternProperties:
   "^(ethernet-)?ports$":
     patternProperties:
       "^(ethernet-)?port@[0-9]+$":
+        $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
         allOf:
           - if:
               properties:
diff --git a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
index c472050582be..546939ab2053 100644
--- a/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/qca8k.yaml
@@ -114,6 +114,15 @@ required:
   - compatible
   - reg
 
+if:
+  required: [ mdio ]
+then:
+  patternProperties:
+    "^(ethernet-)?ports$":
+      patternProperties:
+        "^(ethernet-)?port@[0-6]$":
+          $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
 unevaluatedProperties: false
 
 examples:
diff --git a/Documentation/devicetree/bindings/net/dsa/realtek.yaml b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
index 62ebaa4b5ae3..ec0428b74157 100644
--- a/Documentation/devicetree/bindings/net/dsa/realtek.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/realtek.yaml
@@ -6,8 +6,7 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Realtek switches for unmanaged switches
 
-allOf:
-  - $ref: dsa.yaml#/$defs/ethernet-ports
+$ref: dsa.yaml#/$defs/ethernet-ports
 
 maintainers:
   - Linus Walleij <linus.walleij@linaro.org>
@@ -107,33 +106,43 @@ properties:
       compatible:
         const: realtek,smi-mdio
 
-if:
-  required:
-    - reg
+allOf:
+  - if:
+      required:
+        - reg
 
-then:
-  $ref: /schemas/spi/spi-peripheral-props.yaml#
-  properties:
-    mdc-gpios: false
-    mdio-gpios: false
-    mdio:
+    then:
+      $ref: /schemas/spi/spi-peripheral-props.yaml#
       properties:
-        compatible: false
+        mdc-gpios: false
+        mdio-gpios: false
+        mdio:
+          properties:
+            compatible: false
 
-else:
-  properties:
-    mdio:
-      required:
-        - compatible
+    else:
+      properties:
+        mdio:
+          required:
+            - compatible
 
-  required:
-    - mdc-gpios
-    - mdio-gpios
-    - mdio
-    - reset-gpios
+      required:
+        - mdc-gpios
+        - mdio-gpios
+        - mdio
+        - reset-gpios
 
-required:
-  - compatible
+    required:
+      - compatible
+
+  - if:
+      required: [ mdio ]
+    then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          patternProperties:
+            "^(ethernet-)?port@[0-9]+$":
+              $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
 
 unevaluatedProperties: false
 
diff --git a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
index 9ad9f5cdf688..7ec4371932ef 100644
--- a/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
@@ -72,7 +72,7 @@ properties:
 
     patternProperties:
       "^(ethernet-)?port@[0-4]$":
-        type: object
+        $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
         description: Ethernet switch ports
 
         properties:
diff --git a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
index c80c880a9dab..c31db5f7d548 100644
--- a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
+++ b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
@@ -136,6 +136,7 @@ dependencies:
 
 allOf:
   - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
   - if:
       properties:
         compatible:
diff --git a/Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml b/Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml
index a1b71b35319e..b2cab0fa7b35 100644
--- a/Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml
+++ b/Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml
@@ -15,6 +15,7 @@ description:
 
 allOf:
   - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/net/marvell,pp2.yaml b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
index 4eadafc43d4f..8e60cc4bbbc9 100644
--- a/Documentation/devicetree/bindings/net/marvell,pp2.yaml
+++ b/Documentation/devicetree/bindings/net/marvell,pp2.yaml
@@ -59,7 +59,9 @@ patternProperties:
   '^(ethernet-)?port@[0-2]$':
     type: object
     description: subnode for each ethernet port.
-    $ref: ethernet-controller.yaml#
+    allOf:
+      - $ref: ethernet-controller.yaml#
+      - $ref: ethernet-controller.yaml#/$defs/phylink
     unevaluatedProperties: false
 
     properties:
diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
index e74502a0afe8..7562bb64baa9 100644
--- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
+++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
@@ -391,6 +391,7 @@ patternProperties:
     unevaluatedProperties: false
     allOf:
       - $ref: ethernet-controller.yaml#
+      - $ref: ethernet-controller.yaml#/$defs/phylink
     description:
       Ethernet MAC node
     properties:
diff --git a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
index 306ef9ecf2b9..a770cfc490a9 100644
--- a/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
+++ b/Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
@@ -73,7 +73,9 @@ properties:
       "^port@[0-9a-f]+$":
         type: object
 
-        $ref: /schemas/net/ethernet-controller.yaml#
+        allOf:
+          - $ref: ethernet-controller.yaml#
+          - $ref: ethernet-controller.yaml#/$defs/phylink
         unevaluatedProperties: false
 
         properties:
diff --git a/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml b/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml
index fcafef8d5a33..da4b4d934705 100644
--- a/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml
+++ b/Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml
@@ -84,7 +84,10 @@ properties:
 
     patternProperties:
       "^port@[0-9a-f]+$":
-        $ref: /schemas/net/ethernet-controller.yaml#
+        type: object
+        allOf:
+          - $ref: ethernet-controller.yaml#
+          - $ref: ethernet-controller.yaml#/$defs/phylink
         unevaluatedProperties: false
 
         properties:
diff --git a/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml b/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
index 8ee2c7d7ff42..b3aff7b0790e 100644
--- a/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
+++ b/Documentation/devicetree/bindings/net/mscc,vsc7514-switch.yaml
@@ -126,6 +126,11 @@ properties:
       - const: xtr
       - const: fdma
 
+  ethernet-ports:
+    patternProperties:
+      "^port@[0-9a-f]+$":
+        $ref: ethernet-controller.yaml#/$defs/phylink
+
 required:
   - compatible
   - reg
diff --git a/Documentation/devicetree/bindings/net/qca,ar71xx.yaml b/Documentation/devicetree/bindings/net/qca,ar71xx.yaml
index 89f94b31b546..d021bfb4ad20 100644
--- a/Documentation/devicetree/bindings/net/qca,ar71xx.yaml
+++ b/Documentation/devicetree/bindings/net/qca,ar71xx.yaml
@@ -8,6 +8,7 @@ title: QCA AR71XX MAC
 
 allOf:
   - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
 
 maintainers:
   - Oleksij Rempel <o.rempel@pengutronix.de>
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index ddf9522a5dc2..62d8c2cfafe9 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -567,6 +567,7 @@ dependencies:
 
 allOf:
   - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
   - if:
       properties:
         compatible:
diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
index c9c25132d154..d4366a11918e 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -117,7 +117,9 @@ properties:
         type: object
         description: CPSWxG NUSS external ports
 
-        $ref: ethernet-controller.yaml#
+        allOf:
+          - $ref: ethernet-controller.yaml#
+          - $ref: ethernet-controller.yaml#/$defs/phylink
         unevaluatedProperties: false
 
         properties:
diff --git a/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml b/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
index 1d33d80af11c..f8894546c02a 100644
--- a/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
+++ b/Documentation/devicetree/bindings/net/xlnx,axi-ethernet.yaml
@@ -130,7 +130,8 @@ required:
   - phy-handle
 
 allOf:
-  - $ref: /schemas/net/ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
 
 additionalProperties: false
 
-- 
2.39.2

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Rob Herring]

On Sat, 16 Sep 2023 14:08:59 +0300, Arınç ÜNAL wrote:
> Phylink bindings are required for ethernet controllers that utilise
> phylink_fwnode_phy_connect() directly or through phylink_of_phy_connect(),
> and register OF-based only MDIO buses, if they register any.
> 
> All the drivers that utilise phylink_fwnode_phy_connect() directly or
> through phylink_of_phy_connect():
> 
> - DSA
> - drivers/net/ethernet/mscc/ocelot_net.c
>   - mscc,vsc7514-switch.yaml
> - drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
>   - microchip,sparx5-switch.yaml
> - drivers/net/ethernet/altera/altera_tse_main.c
>   - altr,tse.yaml
> - drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>   - xlnx,axi-ethernet.yaml
> - drivers/net/ethernet/mediatek/mtk_eth_soc.c
>   - mediatek,net.yaml
> - drivers/net/ethernet/ti/am65-cpsw-nuss.c
>   - ti,k3-am654-cpsw-nuss.yaml
> - drivers/net/ethernet/atheros/ag71xx.c
>   - qca,ar71xx.yaml
> - drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
>   - fsl,fman-dtsec.yaml
> - drivers/net/ethernet/microchip/lan966x/lan966x_main.c
>   - microchip,lan966x-switch.yaml
> - drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>   - marvell,pp2.yaml
> - drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
>   - fsl,qoriq-mc-dpmac.yaml
> - drivers/net/ethernet/cadence/macb_main.c
>   - cdns,macb.yaml
>   - Can register non-OF-based bus.
> - drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>   - snps,dwmac.yaml
>   - Can register non-OF-based bus.
> - drivers/net/ethernet/marvell/mvneta.c
>   - marvell-armada-370-neta.txt
> - drivers/net/ethernet/freescale/enetc/enetc.c
>   - fsl-enetc.txt
> 
> RFC: The drivers marked with "can register non-OF-based bus" seem to search
> the MDIO bus to connect the PHY to the MAC using phylink_connect_phy()
> and/or phy_find_first() if phylink bindings don't exist. Should we enforce
> phylink bindings on their schemas regardless?
> 
> DSA, like any other driver utilising the Linux MDIO infrastructure,
> can register a bus. On certain conditions, it registers the MDIO
> bus of the switch it controls non-OF-based.
> 
> We can know when DSA won't create any non-OF-based buses. That leaves us
> with only OF-based buses in which case we can enforce phylink bindings for
> user ports. The bindings are already enforced for shared ports so we can
> enforce the bindings for all ports on the switch schemas which will achieve
> the same result with fewer lines.
> 
> By looking at whether the mdio child node exists and what is explained on
> it, we can enforce phylink bindings.
> 
> - mscc,vsc7514-switch.yaml
>   - Enforce phylink bindings for all ports.
> 
> - arrow,xrs700x.yaml
>   - Enforce phylink bindings for all ports.
> 
> - brcm,b53.yaml
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - brcm,sf2.yaml
>   - Enforce phylink bindings for all ports.
> 
> - hirschmann,hellcreek.yaml
>   - Enforce phylink bindings for all ports.
> 
> - mediatek,mt7530.yaml
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - microchip,ksz.yaml
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - microchip,lan937x.yaml
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - mscc,ocelot.yaml
>   - Enforce phylink bindings for all ports.
> 
> - nxp,sja1105.yaml
>   - Enforce phylink bindings for all ports.
> 
> - qca8k.yaml
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - realtek.yaml
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - renesas,rzn1-a5psw.yaml
>   - Enforce phylink bindings for all ports.
> 
> - ar9331.txt
>   - Enforce phylink bindings for all ports.
> 
> - lan9303.txt
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - lantiq-gswip.txt
>   - Enforce phylink bindings for all ports.
> 
> - marvell.txt
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> - vitesse,vsc73xx.txt
>   - Enforce phylink bindings for all ports if "mdio" is defined.
> 
> I will convert the non json-schema documents accordingly.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../devicetree/bindings/net/altr,tse.yaml     |  1 +
>  .../devicetree/bindings/net/cdns,macb.yaml    |  1 +
>  .../bindings/net/dsa/arrow,xrs700x.yaml       |  6 ++
>  .../devicetree/bindings/net/dsa/brcm,b53.yaml |  9 +++
>  .../devicetree/bindings/net/dsa/brcm,sf2.yaml |  6 +-
>  .../net/dsa/hirschmann,hellcreek.yaml         |  6 ++
>  .../bindings/net/dsa/mediatek,mt7530.yaml     |  9 +++
>  .../bindings/net/dsa/microchip,ksz.yaml       |  9 +++
>  .../bindings/net/dsa/microchip,lan937x.yaml   |  9 +++
>  .../bindings/net/dsa/mscc,ocelot.yaml         |  6 ++
>  .../bindings/net/dsa/nxp,sja1105.yaml         |  1 +
>  .../devicetree/bindings/net/dsa/qca8k.yaml    |  9 +++
>  .../devicetree/bindings/net/dsa/realtek.yaml  | 57 +++++++++++--------
>  .../bindings/net/dsa/renesas,rzn1-a5psw.yaml  |  2 +-
>  .../bindings/net/fsl,fman-dtsec.yaml          |  1 +
>  .../bindings/net/fsl,qoriq-mc-dpmac.yaml      |  1 +
>  .../devicetree/bindings/net/marvell,pp2.yaml  |  4 +-
>  .../devicetree/bindings/net/mediatek,net.yaml |  1 +
>  .../net/microchip,lan966x-switch.yaml         |  4 +-
>  .../bindings/net/microchip,sparx5-switch.yaml |  5 +-
>  .../bindings/net/mscc,vsc7514-switch.yaml     |  5 ++
>  .../devicetree/bindings/net/qca,ar71xx.yaml   |  1 +
>  .../devicetree/bindings/net/snps,dwmac.yaml   |  1 +
>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   |  4 +-
>  .../bindings/net/xlnx,axi-ethernet.yaml       |  3 +-
>  25 files changed, 130 insertions(+), 31 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@5800a000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@5800a000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts', 'phy-mode', 'snps,axi-config', 'snps,pbl', 'snps,tso' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@5800a000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/snps,dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@40028000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@40028000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts', 'phy-mode', 'snps,mixed-burst', 'snps,pbl' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@40028000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/snps,dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@40027000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@40027000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts', 'phy-mode', 'snps,pbl' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@40027000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/snps,dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/cdns,macb.example.dtb: ethernet@fffc4000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/cdns,macb.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell,pp2.example.dtb: ethernet@f0000: ethernet-port@0: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell,pp2.example.dtb: ethernet@f0000: ethernet-port@1: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell,pp2.example.dtb: ethernet@0: ethernet-port@0: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell,pp2.example.dtb: ethernet@0: ethernet-port@1: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/marvell,pp2.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nxp,dwmac-imx.example.dtb: ethernet@30bf0000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/nxp,dwmac-imx.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nxp,dwmac-imx.example.dtb: ethernet@30bf0000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts', 'phy-mode', 'reg' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/nxp,dwmac-imx.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/nxp,dwmac-imx.example.dtb: ethernet@30bf0000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/snps,dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/rockchip-dwmac.example.dtb: ethernet@ff290000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/rockchip-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/rockchip-dwmac.example.dtb: ethernet@ff290000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts', 'phy-mode', 'reg' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/rockchip-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dtb: ethernet@c9410000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/amlogic,meson-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dtb: ethernet@c9410000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts', 'phy-mode' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/amlogic,meson-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.example.dtb: ethernet@c9410000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/snps,dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/snps,dwmac.example.dtb: ethernet@e0800000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/snps,dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun7i-a20-gmac.example.dtb: ethernet@1c50000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/allwinner,sun7i-a20-gmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun7i-a20-gmac.example.dtb: ethernet@1c50000: Unevaluated properties are not allowed ('phy-mode' was unexpected)
	from schema $id: http://devicetree.org/schemas/net/allwinner,sun7i-a20-gmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mediatek-dwmac.example.dtb: ethernet@1101c000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/snps,dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mediatek-dwmac.example.dtb: ethernet@1101c000: 'anyOf' conditional failed, one must be fixed:
	'fixed-link' is a required property
	'pcs-handle' is a required property
	'tbi-handle' is a required property
	'phy-handle' is a required property
	'sfp' is a required property
	'managed' is a required property
	from schema $id: http://devicetree.org/schemas/net/mediatek-dwmac.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/mediatek-dwmac.example.dtb: ethernet@1101c000: Unevaluated properties are not allowed ('interrupt-names', 'interrupts', 'mac-address', 'phy-mode', 'reg', 'snps,reset-delays-us', 'snps,reset-gpio', 'snps,rxpbl', 'snps,txpbl' were unexpected)
	from schema $id: http://devicetree.org/schemas/net/mediatek-dwmac.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230916110902.234273-8-arinc.unal@arinc9.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Arınç ÜNAL]

On 16.09.2023 17:58, Rob Herring wrote:
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@5800a000: 'anyOf' conditional failed, one must be fixed:
> 	'fixed-link' is a required property
> 	'pcs-handle' is a required property
> 	'tbi-handle' is a required property
> 	'phy-handle' is a required property
> 	'sfp' is a required property
> 	'managed' is a required property
> 	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#

I will address the examples once I get a comment on my paragraph starting
with "RFC:" on the patch log.

Arınç

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Rob Herring]

On Sat, Sep 16, 2023 at 02:08:59PM +0300, Arınç ÜNAL wrote:
> Phylink bindings are required for ethernet controllers that utilise
> phylink_fwnode_phy_connect() directly or through phylink_of_phy_connect(),
> and register OF-based only MDIO buses, if they register any.

What is phylink?

Don't describe/justify binding changes based on some Linux functions.

> All the drivers that utilise phylink_fwnode_phy_connect() directly or
> through phylink_of_phy_connect():
> 
> - DSA
> - drivers/net/ethernet/mscc/ocelot_net.c
>   - mscc,vsc7514-switch.yaml
> - drivers/net/ethernet/microchip/sparx5/sparx5_netdev.c
>   - microchip,sparx5-switch.yaml
> - drivers/net/ethernet/altera/altera_tse_main.c
>   - altr,tse.yaml
> - drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>   - xlnx,axi-ethernet.yaml
> - drivers/net/ethernet/mediatek/mtk_eth_soc.c
>   - mediatek,net.yaml
> - drivers/net/ethernet/ti/am65-cpsw-nuss.c
>   - ti,k3-am654-cpsw-nuss.yaml
> - drivers/net/ethernet/atheros/ag71xx.c
>   - qca,ar71xx.yaml
> - drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
>   - fsl,fman-dtsec.yaml
> - drivers/net/ethernet/microchip/lan966x/lan966x_main.c
>   - microchip,lan966x-switch.yaml
> - drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
>   - marvell,pp2.yaml
> - drivers/net/ethernet/freescale/dpaa2/dpaa2-mac.c
>   - fsl,qoriq-mc-dpmac.yaml
> - drivers/net/ethernet/cadence/macb_main.c
>   - cdns,macb.yaml
>   - Can register non-OF-based bus.
> - drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>   - snps,dwmac.yaml
>   - Can register non-OF-based bus.
> - drivers/net/ethernet/marvell/mvneta.c
>   - marvell-armada-370-neta.txt
> - drivers/net/ethernet/freescale/enetc/enetc.c
>   - fsl-enetc.txt
> 
> RFC: The drivers marked with "can register non-OF-based bus" seem to search
> the MDIO bus to connect the PHY to the MAC using phylink_connect_phy()
> and/or phy_find_first() if phylink bindings don't exist. Should we enforce
> phylink bindings on their schemas regardless?

Generally, describing the MDIO bus in DT is optional because the devices 
on the bus can be discovered. But then sometimes a device can't be 
discovered or has additional properties which aren't discoverable. So in 
general, an MDIO bus in DT should always be optional, but always 
supported (and validated) if present. If the device has a separate node 
for the MDIO controller (i.e. one with a compatible and reg for the MDIO 
controller register), then that should always be there (because the h/w 
is always there).

Rob

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Arınç ÜNAL]

On 18.09.2023 21:13, Rob Herring wrote:
> On Sat, Sep 16, 2023 at 02:08:59PM +0300, Arınç ÜNAL wrote:
>> Phylink bindings are required for ethernet controllers that utilise
>> phylink_fwnode_phy_connect() directly or through phylink_of_phy_connect(),
>> and register OF-based only MDIO buses, if they register any.
> 
> What is phylink?
> 
> Don't describe/justify binding changes based on some Linux functions.

I'd like to discuss the influence of the Linux drivers on the devicetree
bindings. Before that let me clarify these "phylink" bindings. They are
certain rules to properly describe the link of the ethernet controller. Any
of the phy-handle, pcs-handle, sfp, and fixed-link properties describe the
link. They are already defined on ethernet-controller.yaml, just not
enforced.

Why I called them phylink bindings, is because when phylink is used on the
ethernet controller driver, only then we can be sure that the link
descriptions on the ethernet controller are needed. Because, some drivers
don't need the link descriptions on the ethernet controller as they can
search a certain MDIO bus to find and connect a MAC to a PHY or set up a
fixed link without looking at the devicetree.

I don't like this approach because, as you said, devicetree bindings should
describe the hardware, not driver policies. So I propose that we don't let
Linux drivers interfere with dt-bindings. To that extent:

- Link descriptions must be required on ethernet controllers. We don't care
   whether some Linux driver can or cannot find the PHY or set up a fixed
   link without looking at the devicetree.

- The MDIO bus description on an ethernet controller describes that the
   hardware comes with an MDIO bus. The MDIO property is optional not
   because some Linux driver can find and map the PHYs to MACs without DT
   descriptions but because the hardware doesn't have to use that or any
   MDIO bus.

Let's also talk about the elephant in the room: DSA bindings. Distributed
Switch Architecture is a Linux subsystem, not a hardware property.
Devicetree bindings are supposed to describe the hardware, yet we have
bindings specifically for DSA so that the DSA driver can properly control
the hardware.

Although I see dsa.yaml and dsa-port.yaml mostly consist of describing an
ethernet switch with CPU port(s), there're properties that are specific to
DSA, such as dsa,member on dsa.yaml and dsa-tag-protocol and label on
dsa-port.yaml.

Depending on how I get responses, I will address my patches. Then I can
further discuss the DSA bindings.

Arınç

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Andrew Lunn]

> - Link descriptions must be required on ethernet controllers. We don't care
>   whether some Linux driver can or cannot find the PHY or set up a fixed
>   link without looking at the devicetree.

That can lead to future surprises, and breakage.

Something which is not used is not tested, and so sometimes wrong, and
nobody knows. Say the driver is extended to a new device and actually
does need to use this never before used information. You then find it
is wrong, and you get a regression.

We have had issues like this before. There are four rgmii phy-link
modes. We have had PHY drivers which ignored one of those modes, it
silently accepted it, but did not change the hardware to actually use
that mode. The PHY continues to use its reset defaults or strapping,
and it worked. A lot of device trees ended up using this mode. And it
was not the same as reset defaults/strapping.

And then somebody needed that fourth mode, and made it actually
work. And all those boards wrongly using that mode broke.

The lesson i learned from that episode is that anything in device tree
must actually be used and tested.

> Although I see dsa.yaml and dsa-port.yaml mostly consist of describing an
> ethernet switch with CPU port(s), there're properties that are specific to
> DSA, such as dsa,member on dsa.yaml and dsa-tag-protocol and label on
> dsa-port.yaml.

I would say dsa,member does describe the hardware. It provides two
bits of information:

Which cluster of switches does this switch belong to. You probably can
derive it using the DSA links between switches, which is also a
hardware property. But having it in device tree makes it simpler.

Which switch is this within a cluster. You need to be able to say:
Send this frame out Port X of switch Y. How does a switch know it is
Y? It could be strapping, which is clearly a hardware property.

dsa-tag-protocol is similar to phy-mode. It tells you the protocol
running between the CPU port and the SoC master interface. You often
can imply it, but again, it could be determined by strapping on the
switch.

label is an interesting one, and probably would not be accepted if it
was proposed now. But it has been around a long time. It also does
describe the hardware, it is what is printed on the case next to the
RJ45. To make the user experience simpler, we then try to make the
linux interface name match the label on the case.

	Andrew

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Arınç ÜNAL]

On 21.09.2023 16:00, Andrew Lunn wrote:
>> - Link descriptions must be required on ethernet controllers. We don't care
>>    whether some Linux driver can or cannot find the PHY or set up a fixed
>>    link without looking at the devicetree.
> 
> That can lead to future surprises, and breakage.
> 
> Something which is not used is not tested, and so sometimes wrong, and
> nobody knows. Say the driver is extended to a new device and actually
> does need to use this never before used information. You then find it
> is wrong, and you get a regression.
> 
> We have had issues like this before. There are four rgmii phy-link
> modes. We have had PHY drivers which ignored one of those modes, it
> silently accepted it, but did not change the hardware to actually use
> that mode. The PHY continues to use its reset defaults or strapping,
> and it worked. A lot of device trees ended up using this mode. And it
> was not the same as reset defaults/strapping.
> 
> And then somebody needed that fourth mode, and made it actually
> work. And all those boards wrongly using that mode broke.
> 
> The lesson i learned from that episode is that anything in device tree
> must actually be used and tested.

It looks like the root cause here was the lack of dt-bindings to only allow
the phy-mode values the hardware supports. What I see here is the driver
change should've been tested on all different hardware the driver controls
then the improper describing of hardware on the devicetree source file
addressed.

If a devicetree change that ensures proper describing of hardware is found
to break a driver in the future, then that exposes a bug on the driver and
the driver will have to be fixed. I don't see this upholding writing
dt-bindings that ensures proper describing of the hardware.

> 
>> Although I see dsa.yaml and dsa-port.yaml mostly consist of describing an
>> ethernet switch with CPU port(s), there're properties that are specific to
>> DSA, such as dsa,member on dsa.yaml and dsa-tag-protocol and label on
>> dsa-port.yaml.
> 
> I would say dsa,member does describe the hardware. It provides two
> bits of information:
> 
> Which cluster of switches does this switch belong to. You probably can
> derive it using the DSA links between switches, which is also a
> hardware property. But having it in device tree makes it simpler.
> 
> Which switch is this within a cluster. You need to be able to say:
> Send this frame out Port X of switch Y. How does a switch know it is
> Y? It could be strapping, which is clearly a hardware property.
> 
> dsa-tag-protocol is similar to phy-mode. It tells you the protocol
> running between the CPU port and the SoC master interface. You often
> can imply it, but again, it could be determined by strapping on the
> switch.
> 
> label is an interesting one, and probably would not be accepted if it
> was proposed now. But it has been around a long time. It also does
> describe the hardware, it is what is printed on the case next to the
> RJ45. To make the user experience simpler, we then try to make the
> linux interface name match the label on the case.

Looks like we can incorporate dsa.port and dsa-port.yaml into
ethernet-switch.yaml and ethernet-switch-port.yaml with adjustments.

Arınç

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Andrew Lunn]

On Thu, Sep 21, 2023 at 09:21:40PM +0300, Arınç ÜNAL wrote:
> On 21.09.2023 16:00, Andrew Lunn wrote:
> > > - Link descriptions must be required on ethernet controllers. We don't care
> > >    whether some Linux driver can or cannot find the PHY or set up a fixed
> > >    link without looking at the devicetree.
> > 
> > That can lead to future surprises, and breakage.
> > 
> > Something which is not used is not tested, and so sometimes wrong, and
> > nobody knows. Say the driver is extended to a new device and actually
> > does need to use this never before used information. You then find it
> > is wrong, and you get a regression.
> > 
> > We have had issues like this before. There are four rgmii phy-link
> > modes. We have had PHY drivers which ignored one of those modes, it
> > silently accepted it, but did not change the hardware to actually use
> > that mode. The PHY continues to use its reset defaults or strapping,
> > and it worked. A lot of device trees ended up using this mode. And it
> > was not the same as reset defaults/strapping.
> > 
> > And then somebody needed that fourth mode, and made it actually
> > work. And all those boards wrongly using that mode broke.
> > 
> > The lesson i learned from that episode is that anything in device tree
> > must actually be used and tested.
> 
> It looks like the root cause here was the lack of dt-bindings to
> only allow the phy-mode values the hardware supports.

That would not help. The hardware supported all 4 RGMII modes. So
listing all four in the dt-binding would be correct. But the driver
for the hardware had a bug, and so silently ignored one of the
modes. That then masked the bugs in board DT files.

> What I see here is the driver change should've been tested on all
> different hardware the driver controls then the improper describing
> of hardware on the devicetree source file addressed.

Which is what did happen. But it took a while to find all those broken
boards.  For a period of time, we had regressions.

Bugs happen. It is a fact of life. But we want those bugs to be easy
to find as possible. If we force DT writers to add properties which
the driver never uses, they are going to be bugs in those
properties. And those bugs are not going to be easy to find, and quite
likely, they will only be found a long time after they are added. We
should not be adding unused properties and bugs just to keep a yaml
checker happy.

	Andrew

Re: [PATCH net-next v2 07/10] dt-bindings: net: enforce phylink bindings on certain ethernet controllers

[Arınç ÜNAL]

On 22.09.2023 02:29, Andrew Lunn wrote:
> On Thu, Sep 21, 2023 at 09:21:40PM +0300, Arınç ÜNAL wrote:
>> On 21.09.2023 16:00, Andrew Lunn wrote:
>>>> - Link descriptions must be required on ethernet controllers. We don't care
>>>>     whether some Linux driver can or cannot find the PHY or set up a fixed
>>>>     link without looking at the devicetree.
>>>
>>> That can lead to future surprises, and breakage.
>>>
>>> Something which is not used is not tested, and so sometimes wrong, and
>>> nobody knows. Say the driver is extended to a new device and actually
>>> does need to use this never before used information. You then find it
>>> is wrong, and you get a regression.
>>>
>>> We have had issues like this before. There are four rgmii phy-link
>>> modes. We have had PHY drivers which ignored one of those modes, it
>>> silently accepted it, but did not change the hardware to actually use
>>> that mode. The PHY continues to use its reset defaults or strapping,
>>> and it worked. A lot of device trees ended up using this mode. And it
>>> was not the same as reset defaults/strapping.
>>>
>>> And then somebody needed that fourth mode, and made it actually
>>> work. And all those boards wrongly using that mode broke.
>>>
>>> The lesson i learned from that episode is that anything in device tree
>>> must actually be used and tested.
>>
>> It looks like the root cause here was the lack of dt-bindings to
>> only allow the phy-mode values the hardware supports.
> 
> That would not help. The hardware supported all 4 RGMII modes. So
> listing all four in the dt-binding would be correct. But the driver
> for the hardware had a bug, and so silently ignored one of the
> modes. That then masked the bugs in board DT files.
> 
>> What I see here is the driver change should've been tested on all
>> different hardware the driver controls then the improper describing
>> of hardware on the devicetree source file addressed.
> 
> Which is what did happen. But it took a while to find all those broken
> boards.  For a period of time, we had regressions.
> 
> Bugs happen. It is a fact of life. But we want those bugs to be easy
> to find as possible. If we force DT writers to add properties which
> the driver never uses, they are going to be bugs in those
> properties. And those bugs are not going to be easy to find, and quite
> likely, they will only be found a long time after they are added. We
> should not be adding unused properties and bugs just to keep a yaml
> checker happy.

Understood. Then I will refrain from enforcing link descriptions altogether
as I can't justify enforcing them on ethernet controllers selectively
because certain Linux drivers don't make use of the link properties and may
expose bugs if used.

The link properties are still allowed for all ethernet controllers though,
just not enforced. But I suppose it's better to optionally find the bug
instead of causing a regression then finding the bug.

Arınç

[PATCH net-next v2 08/10] dt-bindings: net: dsa: marvell: convert to json-schema

[Arınç ÜNAL]

Convert the document for Marvell ethernet switches to json-schema.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../devicetree/bindings/net/dsa/marvell.txt   | 109 ----------
 .../devicetree/bindings/net/dsa/marvell.yaml  | 204 ++++++++++++++++++
 2 files changed, 204 insertions(+), 109 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.txt
 create mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.yaml

diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
deleted file mode 100644
index 6ec0c181b6db..000000000000
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ /dev/null
@@ -1,109 +0,0 @@
-Marvell DSA Switch Device Tree Bindings
----------------------------------------
-
-WARNING: This binding is currently unstable. Do not program it into a
-FLASH never to be changed again. Once this binding is stable, this
-warning will be removed.
-
-If you need a stable binding, use the old dsa.txt binding.
-
-Marvell Switches are MDIO devices. The following properties should be
-placed as a child node of an mdio device.
-
-The properties described here are those specific to Marvell devices.
-Additional required and optional properties can be found in dsa.txt.
-
-The compatibility string is used only to find an identification register,
-which is at a different MDIO base address in different switch families.
-- "marvell,mv88e6085"	: Switch has base address 0x10. Use with models:
-			  6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165,
-			  6171, 6172, 6175, 6176, 6185, 6240, 6320, 6321,
-			  6341, 6350, 6351, 6352
-- "marvell,mv88e6190"	: Switch has base address 0x00. Use with models:
-			  6190, 6190X, 6191, 6290, 6361, 6390, 6390X
-- "marvell,mv88e6250"	: Switch has base address 0x08 or 0x18. Use with model:
-			  6220, 6250
-
-Required properties:
-- compatible		: Should be one of "marvell,mv88e6085",
-			  "marvell,mv88e6190" or "marvell,mv88e6250" as
-			  indicated above
-- reg			: Address on the MII bus for the switch.
-
-Optional properties:
-
-- reset-gpios		: Should be a gpio specifier for a reset line
-- interrupts		: Interrupt from the switch
-- interrupt-controller	: Indicates the switch is itself an interrupt
-			  controller. This is used for the PHY interrupts.
-#interrupt-cells = <2>	: Controller uses two cells, number and flag
-- eeprom-length		: Set to the length of an EEPROM connected to the
-			  switch. Must be set if the switch can not detect
-			  the presence and/or size of a connected EEPROM,
-			  otherwise optional.
-- mdio			: Container of PHY and devices on the switches MDIO
-			  bus.
-- mdio?		: Container of PHYs and devices on the external MDIO
-			  bus. The node must contains a compatible string of
-			  "marvell,mv88e6xxx-mdio-external"
-
-Example:
-
-	mdio {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		interrupt-parent = <&gpio0>;
-		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-		#interrupt-cells = <2>;
-
-		switch0: switch@0 {
-			compatible = "marvell,mv88e6085";
-			reg = <0>;
-			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
-
-			mdio {
-				#address-cells = <1>;
-				#size-cells = <0>;
-				switch1phy0: switch1phy0@0 {
-					reg = <0>;
-					interrupt-parent = <&switch0>;
-					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
-				};
-			};
-		};
-	};
-
-	mdio {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		interrupt-parent = <&gpio0>;
-		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-		#interrupt-cells = <2>;
-
-		switch0: switch@0 {
-			compatible = "marvell,mv88e6190";
-			reg = <0>;
-			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
-
-			mdio {
-				#address-cells = <1>;
-				#size-cells = <0>;
-				switch1phy0: switch1phy0@0 {
-					reg = <0>;
-					interrupt-parent = <&switch0>;
-					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
-				};
-			};
-
-			mdio1 {
-				compatible = "marvell,mv88e6xxx-mdio-external";
-				#address-cells = <1>;
-				#size-cells = <0>;
-				switch1phy9: switch1phy0@9 {
-					reg = <9>;
-				};
-			};
-		};
-	};
diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.yaml b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
new file mode 100644
index 000000000000..45756b8d74d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
@@ -0,0 +1,204 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/dsa/marvell.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Ethernet Switches
+
+$ref: dsa.yaml#/$defs/ethernet-ports
+
+maintainers:
+  - Andrew Lunn <andrew@lunn.ch>
+  - Arınç ÜNAL <arinc.unal@arinc9.com>
+
+description:
+  Marvell ethernet switches are MDIO devices.
+
+properties:
+  compatible:
+    oneOf:
+      - description: |
+          Switch has base address 0x10. Use with models:
+          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
+          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
+        const: marvell,mv88e6085
+
+      - description: |
+          Switch has base address 0x00. Use with models:
+          6190, 6190X, 6191, 6290, 6361, 6390, 6390X
+        const: marvell,mv88e6190
+
+      - description: |
+          Switch has base address 0x08 or 0x18. Use with models:
+          6220, 6250
+        const: marvell,mv88e6250
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    description: GPIO to be used to reset the whole device
+    maxItems: 1
+
+  "#interrupt-cells":
+    const: 2
+
+  interrupt-controller: true
+
+  interrupts:
+    maxItems: 1
+
+  eeprom-length:
+    description:
+      Set to the length of an EEPROM connected to the switch. Must be set if the
+      switch can not detect the presence and/or size of a connected EEPROM,
+      otherwise optional.
+
+  mdio:
+    description:
+      The optional node for the MDIO bus of the switch. The bus will be
+      registered non-OF-based if this is not defined.
+    $ref: /schemas/net/mdio.yaml#
+
+  mdio-external:
+    description: The externally reachable MDIO bus of the 6390 family switches
+    $ref: /schemas/net/mdio.yaml#
+
+    properties:
+      compatible:
+        const: marvell,mv88e6xxx-mdio-external
+
+    required:
+      - compatible
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          enum:
+            - marvell,mv88e6085
+            - marvell,mv88e6250
+    then:
+      properties:
+        mdio-external: false
+
+  - if:
+      required: [ mdio ]
+    then:
+      patternProperties:
+        "^(ethernet-)?ports$":
+          patternProperties:
+            "^(ethernet-)?port@[0-9]+$":
+              $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        switch@0 {
+            compatible = "marvell,mv88e6085";
+            reg = <0>;
+            reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+
+            ethernet-ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    phy-mode = "rmii";
+                    ethernet = <&fec1>;
+
+                    fixed-link {
+                        speed = <100>;
+                        full-duplex;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "swp1";
+                };
+            };
+        };
+    };
+
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        switch: switch@0 {
+            compatible = "marvell,mv88e6190";
+            reg = <0>;
+            reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+            interrupt-parent = <&gpio3>;
+            interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+
+            ethernet-ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    phy-mode = "rmii";
+                    ethernet = <&fec1>;
+
+                    fixed-link {
+                        speed = <100>;
+                        full-duplex;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    label = "swp1";
+                    phy-handle = <&ethphy1>;
+                };
+
+                port@2 {
+                    reg = <2>;
+                    label = "swp2";
+                    phy-handle = <&ext_ethphy0>;
+                };
+            };
+
+            mdio {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                ethphy1: ethernet-phy@1 {
+                    reg = <1>;
+                    interrupt-parent = <&switch>;
+                    interrupts = <1 IRQ_TYPE_LEVEL_HIGH>;
+                };
+            };
+
+            mdio-external {
+                compatible = "marvell,mv88e6xxx-mdio-external";
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                ext_ethphy0: ethernet-phy@0 {
+                    reg = <0>;
+                    compatible = "ethernet-phy-ieee802.3-c45";
+                };
+            };
+        };
+    };
-- 
2.39.2

Re: [PATCH net-next v2 08/10] dt-bindings: net: dsa: marvell: convert to json-schema

[Andrew Lunn]

> +properties:
> +  compatible:
> +    oneOf:
> +      - description: |
> +          Switch has base address 0x10. Use with models:
> +          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
> +          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
> +        const: marvell,mv88e6085

The .txt file contained:

    The compatibility string is used only to find an identification
    register, which is at a different MDIO base address in different
    switch families.

Please keep this text. There has been a lot of confusion in the past,
lots of circular arguments etc, and it is way too late to do anything
different. So i want to make it clear what the compatible is used for,
juts to find the ID registers, nothing more.

> +
> +      - description: |
> +          Switch has base address 0x00. Use with models:
> +          6190, 6190X, 6191, 6290, 6361, 6390, 6390X

The .txt file is missing the 6191X and 6393X, which belong here. If
you need to respin, please could you add them here.

	Andrew

Re: [PATCH net-next v2 08/10] dt-bindings: net: dsa: marvell: convert to json-schema

[Arınç ÜNAL]

On 16.09.2023 17:05, Andrew Lunn wrote:
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - description: |
>> +          Switch has base address 0x10. Use with models:
>> +          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
>> +          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
>> +        const: marvell,mv88e6085
> 
> The .txt file contained:
> 
>      The compatibility string is used only to find an identification
>      register, which is at a different MDIO base address in different
>      switch families.
> 
> Please keep this text. There has been a lot of confusion in the past,
> lots of circular arguments etc, and it is way too late to do anything
> different. So i want to make it clear what the compatible is used for,
> juts to find the ID registers, nothing more.

Will do.

> 
>> +
>> +      - description: |
>> +          Switch has base address 0x00. Use with models:
>> +          6190, 6190X, 6191, 6290, 6361, 6390, 6390X
> 
> The .txt file is missing the 6191X and 6393X, which belong here. If
> you need to respin, please could you add them here.

Will do.

Arınç

Re: [PATCH net-next v2 08/10] dt-bindings: net: dsa: marvell: convert to json-schema

[Rob Herring]

On Sat, 16 Sep 2023 14:09:00 +0300, Arınç ÜNAL wrote:
> Convert the document for Marvell ethernet switches to json-schema.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../devicetree/bindings/net/dsa/marvell.txt   | 109 ----------
>  .../devicetree/bindings/net/dsa/marvell.yaml  | 204 ++++++++++++++++++
>  2 files changed, 204 insertions(+), 109 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.txt
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell,mvusb.example.dtb: switch@0: ports: '#address-cells' is a required property
	from schema $id: http://devicetree.org/schemas/net/dsa/marvell.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell,mvusb.example.dtb: switch@0: ports: '#size-cells' is a required property
	from schema $id: http://devicetree.org/schemas/net/dsa/marvell.yaml#
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 966, in resolve_fragment
    document = document[part]
               ~~~~~~~~^^^^^^
KeyError: '$defs'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/bin/dt-validate", line 146, in <module>
    sg.check_dtb(filename)
  File "/usr/local/bin/dt-validate", line 96, in check_dtb
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/local/bin/dt-validate", line 89, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 89, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 89, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 84, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/local/bin/dt-validate", line 40, in check_node
    for error in self.validator.iter_errors(node, filter=match_schema_file):
  File "/usr/local/lib/python3.11/dist-packages/dtschema/validator.py", line 391, in iter_errors
    for error in self.DtValidator(sch,
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 414, in if_
    yield from validator.descend(instance, then, schema_path="then")
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 362, in allOf
    yield from validator.descend(instance, subschema, schema_path=index)
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 414, in if_
    yield from validator.descend(instance, then, schema_path="then")
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 25, in patternProperties
    yield from validator.descend(
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 25, in patternProperties
    yield from validator.descend(
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 294, in ref
    scope, resolved = validator.resolver.resolve(ref)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 898, in resolve
    return url, self._remote_cache(url)
                ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 916, in resolve_from_url
    return self.resolve_fragment(document, fragment)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 968, in resolve_fragment
    raise exceptions.RefResolutionError(
jsonschema.exceptions.RefResolutionError: Unresolvable JSON pointer: '$defs/phylink'

doc reference errors (make refcheckdocs):
Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/net/dsa/marvell.txt
MAINTAINERS: Documentation/devicetree/bindings/net/dsa/marvell.txt

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230916110902.234273-9-arinc.unal@arinc9.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH net-next v2 08/10] dt-bindings: net: dsa: marvell: convert to json-schema

[Rob Herring]

On Sat, Sep 16, 2023 at 02:09:00PM +0300, Arınç ÜNAL wrote:
> Convert the document for Marvell ethernet switches to json-schema.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../devicetree/bindings/net/dsa/marvell.txt   | 109 ----------
>  .../devicetree/bindings/net/dsa/marvell.yaml  | 204 ++++++++++++++++++
>  2 files changed, 204 insertions(+), 109 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.txt
>  create mode 100644 Documentation/devicetree/bindings/net/dsa/marvell.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
> deleted file mode 100644
> index 6ec0c181b6db..000000000000
> --- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
> +++ /dev/null
> @@ -1,109 +0,0 @@
> -Marvell DSA Switch Device Tree Bindings
> ----------------------------------------
> -
> -WARNING: This binding is currently unstable. Do not program it into a
> -FLASH never to be changed again. Once this binding is stable, this
> -warning will be removed.
> -
> -If you need a stable binding, use the old dsa.txt binding.
> -
> -Marvell Switches are MDIO devices. The following properties should be
> -placed as a child node of an mdio device.
> -
> -The properties described here are those specific to Marvell devices.
> -Additional required and optional properties can be found in dsa.txt.
> -
> -The compatibility string is used only to find an identification register,
> -which is at a different MDIO base address in different switch families.
> -- "marvell,mv88e6085"	: Switch has base address 0x10. Use with models:
> -			  6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165,
> -			  6171, 6172, 6175, 6176, 6185, 6240, 6320, 6321,
> -			  6341, 6350, 6351, 6352
> -- "marvell,mv88e6190"	: Switch has base address 0x00. Use with models:
> -			  6190, 6190X, 6191, 6290, 6361, 6390, 6390X
> -- "marvell,mv88e6250"	: Switch has base address 0x08 or 0x18. Use with model:
> -			  6220, 6250
> -
> -Required properties:
> -- compatible		: Should be one of "marvell,mv88e6085",
> -			  "marvell,mv88e6190" or "marvell,mv88e6250" as
> -			  indicated above
> -- reg			: Address on the MII bus for the switch.
> -
> -Optional properties:
> -
> -- reset-gpios		: Should be a gpio specifier for a reset line
> -- interrupts		: Interrupt from the switch
> -- interrupt-controller	: Indicates the switch is itself an interrupt
> -			  controller. This is used for the PHY interrupts.
> -#interrupt-cells = <2>	: Controller uses two cells, number and flag
> -- eeprom-length		: Set to the length of an EEPROM connected to the
> -			  switch. Must be set if the switch can not detect
> -			  the presence and/or size of a connected EEPROM,
> -			  otherwise optional.
> -- mdio			: Container of PHY and devices on the switches MDIO
> -			  bus.
> -- mdio?		: Container of PHYs and devices on the external MDIO
> -			  bus. The node must contains a compatible string of
> -			  "marvell,mv88e6xxx-mdio-external"
> -
> -Example:
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		interrupt-parent = <&gpio0>;
> -		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> -		interrupt-controller;
> -		#interrupt-cells = <2>;
> -
> -		switch0: switch@0 {
> -			compatible = "marvell,mv88e6085";
> -			reg = <0>;
> -			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
> -
> -			mdio {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				switch1phy0: switch1phy0@0 {
> -					reg = <0>;
> -					interrupt-parent = <&switch0>;
> -					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> -				};
> -			};
> -		};
> -	};
> -
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		interrupt-parent = <&gpio0>;
> -		interrupts = <27 IRQ_TYPE_LEVEL_LOW>;
> -		interrupt-controller;
> -		#interrupt-cells = <2>;
> -
> -		switch0: switch@0 {
> -			compatible = "marvell,mv88e6190";
> -			reg = <0>;
> -			reset-gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
> -
> -			mdio {
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				switch1phy0: switch1phy0@0 {
> -					reg = <0>;
> -					interrupt-parent = <&switch0>;
> -					interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> -				};
> -			};
> -
> -			mdio1 {
> -				compatible = "marvell,mv88e6xxx-mdio-external";
> -				#address-cells = <1>;
> -				#size-cells = <0>;
> -				switch1phy9: switch1phy0@9 {
> -					reg = <9>;
> -				};
> -			};
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.yaml b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
> new file mode 100644
> index 000000000000..45756b8d74d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dsa/marvell.yaml
> @@ -0,0 +1,204 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dsa/marvell.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell Ethernet Switches
> +
> +$ref: dsa.yaml#/$defs/ethernet-ports
> +
> +maintainers:
> +  - Andrew Lunn <andrew@lunn.ch>
> +  - Arınç ÜNAL <arinc.unal@arinc9.com>
> +
> +description:
> +  Marvell ethernet switches are MDIO devices.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: |
> +          Switch has base address 0x10. Use with models:
> +          6085, 6095, 6097, 6123, 6131, 6141, 6161, 6165, 6171, 6172, 6175,
> +          6176, 6185, 6240, 6320, 6321, 6341, 6350, 6351, 6352
> +        const: marvell,mv88e6085
> +
> +      - description: |
> +          Switch has base address 0x00. Use with models:
> +          6190, 6190X, 6191, 6290, 6361, 6390, 6390X
> +        const: marvell,mv88e6190
> +
> +      - description: |
> +          Switch has base address 0x08 or 0x18. Use with models:
> +          6220, 6250
> +        const: marvell,mv88e6250
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: GPIO to be used to reset the whole device
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  eeprom-length:
> +    description:
> +      Set to the length of an EEPROM connected to the switch. Must be set if the
> +      switch can not detect the presence and/or size of a connected EEPROM,
> +      otherwise optional.
> +
> +  mdio:
> +    description:
> +      The optional node for the MDIO bus of the switch. The bus will be
> +      registered non-OF-based if this is not defined.

That's a detail of the OS behavior.

> +    $ref: /schemas/net/mdio.yaml#

       unevaluatedProperties: false

> +
> +  mdio-external:
> +    description: The externally reachable MDIO bus of the 6390 family switches
> +    $ref: /schemas/net/mdio.yaml#

       unevaluatedProperties: false

> +
> +    properties:
> +      compatible:
> +        const: marvell,mv88e6xxx-mdio-external
> +
> +    required:
> +      - compatible
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - marvell,mv88e6085
> +            - marvell,mv88e6250
> +    then:
> +      properties:
> +        mdio-external: false
> +
> +  - if:
> +      required: [ mdio ]
> +    then:
> +      patternProperties:
> +        "^(ethernet-)?ports$":
> +          patternProperties:
> +            "^(ethernet-)?port@[0-9]+$":
> +              $ref: /schemas/net/ethernet-controller.yaml#/$defs/phylink

This probably doesn't work right. The problem is every node ultimately 
needs a single schema with all possible properties in order to not allow 
undefined properties. This is the reason for the complexity with a $ref 
at each level in schemas for these nodes.

I'm not sure it is worth the complexity to enforce what you want here. 
It may be easier to just always allow phylink properties in 
ethernet-port nodes.  

As this is a switch, it should be referencing ethernet-switch.yaml. 

Rob

[PATCH net-next v2 09/10] MAINTAINERS: update MARVELL 88E6XXX ETHERNET SWITCH document file name

[Arınç ÜNAL]

The dt-bindings document was converted to json-schema. Update the file
name.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 612d6d1dbf36..5d4e6b578c04 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12564,7 +12564,7 @@ MARVELL 88E6XXX ETHERNET SWITCH FABRIC DRIVER
 M:	Andrew Lunn <andrew@lunn.ch>
 L:	netdev@vger.kernel.org
 S:	Maintained
-F:	Documentation/devicetree/bindings/net/dsa/marvell.txt
+F:	Documentation/devicetree/bindings/net/dsa/marvell.yaml
 F:	Documentation/networking/devlink/mv88e6xxx.rst
 F:	drivers/net/dsa/mv88e6xxx/
 F:	include/linux/dsa/mv88e6xxx.h
-- 
2.39.2

[PATCH net-next v2 10/10] dt-bindings: net: marvell-armada-370-neta: convert to json-schema

[Arınç ÜNAL]

Convert the document for Marvell Armada 370 / Armada XP / Armada 3700
Ethernet Controller (NETA) to json-schema.

Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
---
 .../bindings/net/marvell-armada-370-neta.txt  |  50 ---------
 .../bindings/net/marvell-armada-370-neta.yaml | 102 ++++++++++++++++++
 2 files changed, 102 insertions(+), 50 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
 create mode 100644 Documentation/devicetree/bindings/net/marvell-armada-370-neta.yaml

diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
deleted file mode 100644
index 2bf31572b08d..000000000000
--- a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
+++ /dev/null
@@ -1,50 +0,0 @@
-* Marvell Armada 370 / Armada XP / Armada 3700 Ethernet Controller (NETA)
-
-Required properties:
-- compatible: could be one of the following:
-	"marvell,armada-370-neta"
-	"marvell,armada-xp-neta"
-	"marvell,armada-3700-neta"
-	"marvell,armada-ac5-neta"
-- reg: address and length of the register set for the device.
-- interrupts: interrupt for the device
-- phy: See ethernet.txt file in the same directory.
-- phy-mode: See ethernet.txt file in the same directory
-- clocks: List of clocks for this device. At least one clock is
-  mandatory for the core clock. If several clocks are given, then the
-  clock-names property must be used to identify them.
-
-Optional properties:
-- tx-csum-limit: maximum mtu supported by port that allow TX checksum.
-  Value is presented in bytes. If not used, by default 1600B is set for
-  "marvell,armada-370-neta" and 9800B for others.
-- clock-names: List of names corresponding to clocks property; shall be
-  "core" for core clock and "bus" for the optional bus clock.
-- phys: comphy for the ethernet port, see ../phy/phy-bindings.txt
-
-Optional properties (valid only for Armada XP/38x):
-
-- buffer-manager: a phandle to a buffer manager node. Please refer to
-  Documentation/devicetree/bindings/net/marvell-neta-bm.txt
-- bm,pool-long: ID of a pool, that will accept all packets of a size
-  higher than 'short' pool's threshold (if set) and up to MTU value.
-  Obligatory, when the port is supposed to use hardware
-  buffer management.
-- bm,pool-short: ID of a pool, that will be used for accepting
-  packets of a size lower than given threshold. If not set, the port
-  will use a single 'long' pool for all packets, as defined above.
-
-Example:
-
-ethernet@70000 {
-	compatible = "marvell,armada-370-neta";
-	reg = <0x70000 0x2500>;
-	interrupts = <8>;
-	clocks = <&gate_clk 4>;
-	tx-csum-limit = <9800>
-	phy = <&phy0>;
-	phy-mode = "rgmii-id";
-	buffer-manager = <&bm>;
-	bm,pool-long = <0>;
-	bm,pool-short = <1>;
-};
diff --git a/Documentation/devicetree/bindings/net/marvell-armada-370-neta.yaml b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.yaml
new file mode 100644
index 000000000000..9283ab74cdb2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell-armada-370-neta.yaml
@@ -0,0 +1,102 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/marvell-armada-370-neta.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Armada 370 / Armada XP / Armada 3700 Ethernet Controller (NETA)
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+  - $ref: ethernet-controller.yaml#/$defs/phylink
+
+maintainers:
+  - Arınç ÜNAL <arinc.unal@arinc9.com>
+
+properties:
+  compatible:
+    enum:
+      - marvell,armada-370-neta
+      - marvell,armada-xp-neta
+      - marvell,armada-3700-neta
+      - marvell,armada-ac5-neta
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    description:
+      List of clocks for this device. At least one clock is mandatory for the
+      core clock. If several clocks are given, then the clock-names property
+      must be used to identify them.
+
+  tx-csum-limit:
+    description:
+      Maximum mtu supported by port that allow TX checksum. Value is presented
+      in bytes. If not used, by default 1600B is set for
+      "marvell,armada-370-neta" and 9800B for others.
+
+  clock-names:
+    description:
+      List of names corresponding to clocks property; shall be "core" for core
+      clock and "bus" for the optional bus clock.
+
+  phys:
+    description:
+      comphy for the ethernet port, see ../phy/phy-bindings.txt.
+
+if:
+  properties:
+    compatible:
+      enum:
+        - marvell,armada-370-neta
+        - marvell,armada-xp-neta
+then:
+  properties:
+    buffer-manager:
+      description:
+        A phandle to a buffer manager node. Please refer to
+        Documentation/devicetree/bindings/net/marvell-neta-bm.txt.
+
+    bm,pool-long:
+      description:
+        ID of a pool, that will accept all packets of a size higher than 'short'
+        pool's threshold (if set) and up to MTU value. Obligatory, when the port
+        is supposed to use hardware buffer management.
+
+    bm,pool-short:
+      description:
+        ID of a pool, that will be used for accepting packets of a size lower
+        than given threshold. If not set, the port will use a single 'long' pool
+        for all packets, as defined above.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - phy-mode
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    ethernet@70000 {
+        compatible = "marvell,armada-370-neta";
+        reg = <0x70000 0x2500>;
+        interrupts = <8>;
+        clocks = <&gate_clk 4>;
+        tx-csum-limit = <9800>;
+        phy-mode = "rgmii-id";
+        buffer-manager = <&bm>;
+        bm,pool-long = <0>;
+        bm,pool-short = <1>;
+
+        fixed-link {
+            speed = <1000>;
+            full-duplex;
+        };
+    };
-- 
2.39.2

Re: [PATCH net-next v2 10/10] dt-bindings: net: marvell-armada-370-neta: convert to json-schema

[Rob Herring]

On Sat, 16 Sep 2023 14:09:02 +0300, Arınç ÜNAL wrote:
> Convert the document for Marvell Armada 370 / Armada XP / Armada 3700
> Ethernet Controller (NETA) to json-schema.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
>  .../bindings/net/marvell-armada-370-neta.txt  |  50 ---------
>  .../bindings/net/marvell-armada-370-neta.yaml | 102 ++++++++++++++++++
>  2 files changed, 102 insertions(+), 50 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
>  create mode 100644 Documentation/devicetree/bindings/net/marvell-armada-370-neta.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell-armada-370-neta.yaml:
Unresolvable JSON pointer: '$defs/phylink'
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 966, in resolve_fragment
    document = document[part]
               ~~~~~~~~^^^^^^
KeyError: '$defs'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/bin/dt-validate", line 146, in <module>
    sg.check_dtb(filename)
  File "/usr/local/bin/dt-validate", line 96, in check_dtb
    self.check_subtree(dt, subtree, False, "/", "/", filename)
  File "/usr/local/bin/dt-validate", line 89, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 89, in check_subtree
    self.check_subtree(tree, value, disabled, name, fullname + name, filename)
  File "/usr/local/bin/dt-validate", line 84, in check_subtree
    self.check_node(tree, subtree, disabled, nodename, fullname, filename)
  File "/usr/local/bin/dt-validate", line 40, in check_node
    for error in self.validator.iter_errors(node, filter=match_schema_file):
  File "/usr/local/lib/python3.11/dist-packages/dtschema/validator.py", line 391, in iter_errors
    for error in self.DtValidator(sch,
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 414, in if_
    yield from validator.descend(instance, then, schema_path="then")
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 362, in allOf
    yield from validator.descend(instance, subschema, schema_path=index)
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 305, in descend
    for error in self.evolve(schema=schema).iter_errors(instance):
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 288, in iter_errors
    for error in errors:
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/_validators.py", line 294, in ref
    scope, resolved = validator.resolver.resolve(ref)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 898, in resolve
    return url, self._remote_cache(url)
                ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 916, in resolve_from_url
    return self.resolve_fragment(document, fragment)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/jsonschema/validators.py", line 968, in resolve_fragment
    raise exceptions.RefResolutionError(
jsonschema.exceptions.RefResolutionError: Unresolvable JSON pointer: '$defs/phylink'

doc reference errors (make refcheckdocs):
Warning: Documentation/devicetree/bindings/net/marvell-neta-bm.txt references a file that doesn't exist: Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
Documentation/devicetree/bindings/net/marvell-neta-bm.txt: Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230916110902.234273-11-arinc.unal@arinc9.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH net-next v2 10/10] dt-bindings: net: marvell-armada-370-neta: convert to json-schema

[Arınç ÜNAL]

On 16.09.2023 17:58, Rob Herring wrote:
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/marvell-armada-370-neta.yaml:
> Unresolvable JSON pointer: '$defs/phylink'

Not surprising as this is added with a previous patch in this series.

> doc reference errors (make refcheckdocs):
> Warning: Documentation/devicetree/bindings/net/marvell-neta-bm.txt references a file that doesn't exist: Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
> Documentation/devicetree/bindings/net/marvell-neta-bm.txt: Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt

Will address.

Arınç

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Russell King (Oracle)]

On Sat, Sep 16, 2023 at 02:08:52PM +0300, Arınç ÜNAL wrote:
> Hello there.
> 
> This patch series defines phylink bindings and enforces them for the
> ethernet controllers that need them.
> 
> Some schemas had to be changed to properly enforce phylink bindings for all
> of the affected ethernet controllers. Some of the documents of these
> ethernet controllers were non json-schema, which had to be converted.
> 
> I will convert the remaining documents to json-schema while this patch
> series receives reviews.

I can't say that I'm comfortable with this. We appear to be defining
bindings based on software implementation, and a desire for the DT
tooling to enforce what the software implementation wants. Isn't this
against the aims of device tree and device tree binding documentation?
Seems to me like feature-creep.

The bindings that phylink parses are already documented in the
ethernet controller yaml document. Specifically:

- phylink does not parse the phy-mode property, that is left to the
  implementation to pass to phylink, which can implement it any
  which way they choose (and even default to something.)

- phylink does not require a phy property - phylink does expect a PHY
  to be attached, but how that PHY is attached is up to the ethernet
  controller driver. It may call one of the phylink functions that
  parses the phy property, or it may manually supply the phy device to
  phylink. Either way, phylink does not itself require a PHY property.

- phylink does not require a sfp property - this obviously is optional.

So, all in all, ethernet-controller already describes it, and to create
a DT binding document that pretends that phylink requires any of this
stuff is, in my mind, wrong.

DSA requires certain properties by dint of the parsing and setup of
phylink being in generic code - this is not because phylink requires
certain properties, but phylink does require certain information in
order to function correctly.

The issue here is _how_ phylink gets that information, and as I state
above, it _can_ come from DT, but it can also be given that information
manually.

As an example, there are plenty of drivers in the tree which try to
parse a phy node, and if that's not present, they try to see if a PHY
exists at a default# bus address.

We seem to be digging outselves a hole here, where "phylink must have
these properties". No, that is wrong.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

On 22/09/2023 15:40, Russell King (Oracle) wrote:
> On Sat, Sep 16, 2023 at 02:08:52PM +0300, Arınç ÜNAL wrote:
>> Hello there.
>>
>> This patch series defines phylink bindings and enforces them for the
>> ethernet controllers that need them.
>>
>> Some schemas had to be changed to properly enforce phylink bindings for all
>> of the affected ethernet controllers. Some of the documents of these
>> ethernet controllers were non json-schema, which had to be converted.
>>
>> I will convert the remaining documents to json-schema while this patch
>> series receives reviews.
> 
> I can't say that I'm comfortable with this. We appear to be defining
> bindings based on software implementation, and a desire for the DT
> tooling to enforce what the software implementation wants. Isn't this
> against the aims of device tree and device tree binding documentation?
> Seems to me like feature-creep.
> 
> The bindings that phylink parses are already documented in the
> ethernet controller yaml document. Specifically:
> 
> - phylink does not parse the phy-mode property, that is left to the
>    implementation to pass to phylink, which can implement it any
>    which way they choose (and even default to something.)
> 
> - phylink does not require a phy property - phylink does expect a PHY
>    to be attached, but how that PHY is attached is up to the ethernet
>    controller driver. It may call one of the phylink functions that
>    parses the phy property, or it may manually supply the phy device to
>    phylink. Either way, phylink does not itself require a PHY property.
> 
> - phylink does not require a sfp property - this obviously is optional.
> 
> So, all in all, ethernet-controller already describes it, and to create
> a DT binding document that pretends that phylink requires any of this
> stuff is, in my mind, wrong.
> 
> DSA requires certain properties by dint of the parsing and setup of
> phylink being in generic code - this is not because phylink requires
> certain properties, but phylink does require certain information in
> order to function correctly.
> 
> The issue here is _how_ phylink gets that information, and as I state
> above, it _can_ come from DT, but it can also be given that information
> manually.
> 
> As an example, there are plenty of drivers in the tree which try to
> parse a phy node, and if that's not present, they try to see if a PHY
> exists at a default# bus address.
> 
> We seem to be digging outselves a hole here, where "phylink must have
> these properties". No, that is wrong.

I agree. My patch description here failed to explain the actual issue,
which is missing hardware descriptions. Here's what I understand. An
ethernet-controller is a MAC. For the MAC to work properly with its link
partner, at least one of these must be described:
- pointer to a PHY to retrieve link information from the PHY
- pointer to a PCS to retrieve link information from the PCS
- pointer to an SFP to retrieve link information from the SFP
- static link information

Andrew under the discussion of patch 7 said that enforcing this may expose
bugs on MAC drivers that never looked at the devicetree to control the
MAC's link which would cause regressions, implying we should hold back on
enforcing it. I've agreed not to enforce it, not because it is incorrect
description of ethernet controller hardware - I think it is correct - but
because I won't be the one to deal with the regressions when this
dt-bindings change goes through.

I won't also enforce it selectively, as saying "these drivers use
phylink_fwnode_phy_connect() therefore there won't be any bad surprises on
the hardware they control so let's enforce it only for them" is nonsense in
the context of describing hardware.

I will focus on documenting the missing MDIO bus descriptions on certain
ethernet switches and converting ethernet switch documents (maybe ethernet
controllers too) to json-schema. There's the incorrect link descriptions on
dsa-port.yaml as confirmed by Vladimir on the discussion of v1 series so
I'll fix that.

I've also got some ethernet controller rules that I think won't break any
driver so I will submit them as well.

Arınç

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Russell King (Oracle)]

On Sat, Sep 23, 2023 at 12:57:52AM +0300, Arınç ÜNAL wrote:
> On 22/09/2023 15:40, Russell King (Oracle) wrote:
> > On Sat, Sep 16, 2023 at 02:08:52PM +0300, Arınç ÜNAL wrote:
> > > Hello there.
> > > 
> > > This patch series defines phylink bindings and enforces them for the
> > > ethernet controllers that need them.
> > > 
> > > Some schemas had to be changed to properly enforce phylink bindings for all
> > > of the affected ethernet controllers. Some of the documents of these
> > > ethernet controllers were non json-schema, which had to be converted.
> > > 
> > > I will convert the remaining documents to json-schema while this patch
> > > series receives reviews.
> > 
> > I can't say that I'm comfortable with this. We appear to be defining
> > bindings based on software implementation, and a desire for the DT
> > tooling to enforce what the software implementation wants. Isn't this
> > against the aims of device tree and device tree binding documentation?
> > Seems to me like feature-creep.
> > 
> > The bindings that phylink parses are already documented in the
> > ethernet controller yaml document. Specifically:
> > 
> > - phylink does not parse the phy-mode property, that is left to the
> >    implementation to pass to phylink, which can implement it any
> >    which way they choose (and even default to something.)
> > 
> > - phylink does not require a phy property - phylink does expect a PHY
> >    to be attached, but how that PHY is attached is up to the ethernet
> >    controller driver. It may call one of the phylink functions that
> >    parses the phy property, or it may manually supply the phy device to
> >    phylink. Either way, phylink does not itself require a PHY property.
> > 
> > - phylink does not require a sfp property - this obviously is optional.
> > 
> > So, all in all, ethernet-controller already describes it, and to create
> > a DT binding document that pretends that phylink requires any of this
> > stuff is, in my mind, wrong.
> > 
> > DSA requires certain properties by dint of the parsing and setup of
> > phylink being in generic code - this is not because phylink requires
> > certain properties, but phylink does require certain information in
> > order to function correctly.
> > 
> > The issue here is _how_ phylink gets that information, and as I state
> > above, it _can_ come from DT, but it can also be given that information
> > manually.
> > 
> > As an example, there are plenty of drivers in the tree which try to
> > parse a phy node, and if that's not present, they try to see if a PHY
> > exists at a default# bus address.
> > 
> > We seem to be digging outselves a hole here, where "phylink must have
> > these properties". No, that is wrong.
> 
> I agree. My patch description here failed to explain the actual issue,
> which is missing hardware descriptions. Here's what I understand. An
> ethernet-controller is a MAC. For the MAC to work properly with its link
> partner, at least one of these must be described:
> - pointer to a PHY to retrieve link information from the PHY
> - pointer to a PCS to retrieve link information from the PCS
> - pointer to an SFP to retrieve link information from the SFP
> - static link information

What about something like macb? The macb driver:
- attempts to connect a phy using phylink_of_phy_connect()
- if that fails, and there is no phy-handle property, then the driver
  will attempt to find the first PHY to exist on its MII bus, and will
  connect that using phylink_connect_phy().

So, in this case, if we define a phylink binding to require one of a
phy-handle node, pcs node, sfp node or static link information, then
although macb uses phylink, it then doesn't conform to this phylink
binding. (This is the only driver that uses phy_find_first() which
also uses phylink according to my greps, but I haven't checked for
any other games drivers be using.)

The same thing more or less happens with non-phylink drivers. Take a
look at drivers/net/ethernet/microchip/lan743x_main.c, and notice
that it first attempts to get a PHY from DT. If that fails, it
uses phy_find_first(). If that fails, and we have a LAN7431, then
a gigabit full-duplex fixed-link PHY is used instead. So, what macb
is doing with phylink is no different from what other drivers are
doing with phylib - and it's the driver's choice.

The same way that there are multiple drivers that don't do this,
which want a PHY device to be specified in DT if the driver was
bound to a device that was described in DT - there are phylink
and non-phylink drivers that do this.

This is exactly my point - there is *no* *such* *thing* as a phylink
binding. There is the ethernet-controller binding, which phylink
provides the ability for network drivers to optionally use, but
phylink doesn't require anything from any firmware description, except
to attach a SFP interface, or to describe a fixed-link. Everything else
is really up to the ethernet-controller aka MAC driver to decide how it
wants to deal with things.

We currently work around this by the ethernet-controller YAML having
all these properties as optional. Maybe some drivers extend that YAML
and require certain properties - that is their perogative, but that is
the driver's choice, and is a completely separate issue to whether
the driver is using phylink or not.

The real question is how do we want to describe an ethernet controller
and what properties should we be requiring for it (if any). Maybe if we
want to require one of a PHY, PCS, SFP, or fixed-link, maybe we should
have that as a strictly-checked ethernet controller which drivers can
opt into using if that's what they require.

However, to dress this up as "phylink requires xyz, so lets create
a phylink binding description" is just wrong.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Andrew Lunn]

> However, to dress this up as "phylink requires xyz, so lets create
> a phylink binding description" is just wrong.

+1

Also, phylink is a Linux implementation detail. Other OSes using the
binding don't need to have phylink. Yet they can still use the DT
blobs because they should describe the hardware, independent of how
the OS drives that hardware.

    Andrew

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

On 23.09.2023 01:44, Andrew Lunn wrote:
>> However, to dress this up as "phylink requires xyz, so lets create
>> a phylink binding description" is just wrong.
> 
> +1
> 
> Also, phylink is a Linux implementation detail. Other OSes using the
> binding don't need to have phylink. Yet they can still use the DT
> blobs because they should describe the hardware, independent of how
> the OS drives that hardware.

I haven't stated it directly but I've been agreeing to this fact since the
start of the discussion on patch 7.

Arınç

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

On 23.09.2023 01:29, Russell King (Oracle) wrote:
> On Sat, Sep 23, 2023 at 12:57:52AM +0300, Arınç ÜNAL wrote:
>> I agree. My patch description here failed to explain the actual issue,
>> which is missing hardware descriptions. Here's what I understand. An
>> ethernet-controller is a MAC. For the MAC to work properly with its link
>> partner, at least one of these must be described:
>> - pointer to a PHY to retrieve link information from the PHY
>> - pointer to a PCS to retrieve link information from the PCS
>> - pointer to an SFP to retrieve link information from the SFP
>> - static link information
> 
> What about something like macb? The macb driver:
> - attempts to connect a phy using phylink_of_phy_connect()
> - if that fails, and there is no phy-handle property, then the driver
>    will attempt to find the first PHY to exist on its MII bus, and will
>    connect that using phylink_connect_phy().
> 
> So, in this case, if we define a phylink binding to require one of a
> phy-handle node, pcs node, sfp node or static link information, then
> although macb uses phylink, it then doesn't conform to this phylink
> binding. (This is the only driver that uses phy_find_first() which
> also uses phylink according to my greps, but I haven't checked for
> any other games drivers be using.)
> 
> The same thing more or less happens with non-phylink drivers. Take a
> look at drivers/net/ethernet/microchip/lan743x_main.c, and notice
> that it first attempts to get a PHY from DT. If that fails, it
> uses phy_find_first(). If that fails, and we have a LAN7431, then
> a gigabit full-duplex fixed-link PHY is used instead. So, what macb
> is doing with phylink is no different from what other drivers are
> doing with phylib - and it's the driver's choice.
> 
> The same way that there are multiple drivers that don't do this,
> which want a PHY device to be specified in DT if the driver was
> bound to a device that was described in DT - there are phylink
> and non-phylink drivers that do this.
> 
> This is exactly my point - there is *no* *such* *thing* as a phylink
> binding. There is the ethernet-controller binding, which phylink
> provides the ability for network drivers to optionally use, but
> phylink doesn't require anything from any firmware description, except
> to attach a SFP interface, or to describe a fixed-link. Everything else
> is really up to the ethernet-controller aka MAC driver to decide how it
> wants to deal with things.
> 
> We currently work around this by the ethernet-controller YAML having
> all these properties as optional. Maybe some drivers extend that YAML
> and require certain properties - that is their perogative, but that is
> the driver's choice, and is a completely separate issue to whether
> the driver is using phylink or not.
> 
> The real question is how do we want to describe an ethernet controller
> and what properties should we be requiring for it (if any). Maybe if we
> want to require one of a PHY, PCS, SFP, or fixed-link, maybe we should
> have that as a strictly-checked ethernet controller which drivers can
> opt into using if that's what they require.

I'd like to make this clear. We're only talking about deviating from proper
devicetree bindings so that it won't cause too much work or not at all to
fix the incorrect Linux driver policies.

As long as we don't collectively agree on fixing the drivers to work with
proper devicetree bindings, I'd keep the missing ethernet controller
bindings (requiring at least one of PHY, PCS, SFP, fixed-link) as they
currently are on ethernet-controller.yaml, optional. Or rather, I wouldn't
touch anything regarding this as it's nonsensical to change devicetree
bindings because of driver policies.

As you have pointed out with certain examples, once the driver starts
operating out of what the devicetree says, in other words, once the driver
starts guessing the hardware, there's no guarantee it will always guess it
correctly. As seen with the macb driver, the driver assumes that if there's
no phy-handle property, the PHY on its MDIO bus must be used regardless.
But the MAC may be connected to another MAC, PCS or SFP, meaning it doesn't
use the PHY on that bus.

There is also a case for DSA. If there's an implication that the DSA
controlled switch has an MDIO bus (phy_read() and phy_write()), the DSA
driver will connect the switch MACs to the PHYs on the MDIO bus of the
switch, even if there's no description of that MDIO bus on the devicetree.
As unlikely as it is on a real life scenario, there may be a device that
has its switch MACs wired to the PHYs on another MDIO bus.

This is why I've proposed to make the drivers strictly follow what the
devicetree says.

> 
> However, to dress this up as "phylink requires xyz, so lets create
> a phylink binding description" is just wrong.

Agreed.

Arınç

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Andrew Lunn]

> As you have pointed out with certain examples, once the driver starts
> operating out of what the devicetree says, in other words, once the driver
> starts guessing the hardware, there's no guarantee it will always guess it
> correctly.

This is partially a result of history. Some of these drivers are older
than DT. This guessing was sufficient to make them work in the systems
of that time. Some drivers are used when DT is not available, e.g. USB
or PCI devices, or even ACPI.

> There is also a case for DSA. If there's an implication that the DSA
> controlled switch has an MDIO bus (phy_read() and phy_write()), the DSA
> driver will connect the switch MACs to the PHYs on the MDIO bus of the
> switch, even if there's no description of that MDIO bus on the devicetree.
> As unlikely as it is on a real life scenario, there may be a device that
> has its switch MACs wired to the PHYs on another MDIO bus.

> This is why I've proposed to make the drivers strictly follow what the
> devicetree says.

There are mv88e6xxx systems which don't have a DT description, just
platform data. So if you need to make code changes, keep that in mind.

	 Andrew

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Andrew Lunn]

> I agree. My patch description here failed to explain the actual issue,
> which is missing hardware descriptions. Here's what I understand. An
> ethernet-controller is a MAC. For the MAC to work properly with its link
> partner, at least one of these must be described:
> - pointer to a PHY to retrieve link information from the PHY
> - pointer to a PCS to retrieve link information from the PCS
> - pointer to an SFP to retrieve link information from the SFP
> - static link information

You are missing:

- The MAC has firmware driving the PHY, nothing for linux to do.

There are properties in ethernet-controller.yaml the MAC driver would
however like to use such as local-mac-address, max-frame-size,
nvmem-cell-names etc.

	Andrew

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

On 23.09.2023 01:36, Andrew Lunn wrote:
>> I agree. My patch description here failed to explain the actual issue,
>> which is missing hardware descriptions. Here's what I understand. An
>> ethernet-controller is a MAC. For the MAC to work properly with its link
>> partner, at least one of these must be described:
>> - pointer to a PHY to retrieve link information from the PHY
>> - pointer to a PCS to retrieve link information from the PCS
>> - pointer to an SFP to retrieve link information from the SFP
>> - static link information
> 
> You are missing:
> 
> - The MAC has firmware driving the PHY, nothing for linux to do.
> 
> There are properties in ethernet-controller.yaml the MAC driver would
> however like to use such as local-mac-address, max-frame-size,
> nvmem-cell-names etc.

This is interesting. This is clearly a hardware difference of the ethernet
controller.

I believe this fits case 1. There's still an MDIO bus the ethernet
controller uses, there's still a PHY on the MDIO bus which the ethernet
controller uses. The only difference is the firmware of the ethernet
controller controls... What exactly does the firmware control that a Linux
driver would have controlled instead? Just configuring the link settings of
the MAC?

If it's just MAC link settings, I believe it would make sense to add a
property on the ethernet controller dt-bindings to state that the hardware
controls the MAC link settings on its own. This way, we would still
describe the MDIO bus and PHY of the ethernet controller while also
pointing out that the MAC link settings are not up to a driver to control.

Arınç

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Andrew Lunn]

On Sat, Sep 23, 2023 at 09:28:41AM +0300, Arınç ÜNAL wrote:
> On 23.09.2023 01:36, Andrew Lunn wrote:
> > > I agree. My patch description here failed to explain the actual issue,
> > > which is missing hardware descriptions. Here's what I understand. An
> > > ethernet-controller is a MAC. For the MAC to work properly with its link
> > > partner, at least one of these must be described:
> > > - pointer to a PHY to retrieve link information from the PHY
> > > - pointer to a PCS to retrieve link information from the PCS
> > > - pointer to an SFP to retrieve link information from the SFP
> > > - static link information
> > 
> > You are missing:
> > 
> > - The MAC has firmware driving the PHY, nothing for linux to do.
> > 
> > There are properties in ethernet-controller.yaml the MAC driver would
> > however like to use such as local-mac-address, max-frame-size,
> > nvmem-cell-names etc.
> 
> This is interesting. This is clearly a hardware difference of the ethernet
> controller.
> 
> I believe this fits case 1. There's still an MDIO bus the ethernet
> controller uses, there's still a PHY on the MDIO bus which the ethernet
> controller uses.

Why must there be an MDIO bus? All the bus provides is a communication
channel to the PHY. There are PHYs which are memory mapped, or use
I2C. SFP are a good example of I2C, which Linux maps to MDIO just to
make things simple, but the hardware is I2C. Why must there be a PHY?
Maybe it is a Base-K link, i.e. a baseboard link to a switch, or a BMC
or something.

> The only difference is the firmware of the ethernet
> controller controls... What exactly does the firmware control that a Linux
> driver would have controlled instead? Just configuring the link settings of
> the MAC?

A MAC driver implements struct ethtool_ops:::get_link_settings and
set_link_settings. For a MAC driver using phylib or phylink they
typically then call into phylib or phylink to do the actual work,
maybe with a bit of pre-processing in the MAC driver.

A MAC driver using firmware would typically make an RPC into the
firmware to implement these calls.

There is a MAC driver currently under review which does not have a PHY
at all. The MAC is directly connected to a switch, all within one
IC. The link is always running at 5Gbps, the link is always up. It is
physically impossible to connect a PHY, so get_link_settings just
returns hard coded values.

	Andrew

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

On 23.09.2023 18:12, Andrew Lunn wrote:
> On Sat, Sep 23, 2023 at 09:28:41AM +0300, Arınç ÜNAL wrote:
>> On 23.09.2023 01:36, Andrew Lunn wrote:
>>> You are missing:
>>>
>>> - The MAC has firmware driving the PHY, nothing for linux to do.
>>>
>>> There are properties in ethernet-controller.yaml the MAC driver would
>>> however like to use such as local-mac-address, max-frame-size,
>>> nvmem-cell-names etc.
>>
>> This is interesting. This is clearly a hardware difference of the ethernet
>> controller.
>>
>> I believe this fits case 1. There's still an MDIO bus the ethernet
>> controller uses, there's still a PHY on the MDIO bus which the ethernet
>> controller uses.
> 
> Why must there be an MDIO bus? All the bus provides is a communication
> channel to the PHY. There are PHYs which are memory mapped, or use
> I2C. SFP are a good example of I2C, which Linux maps to MDIO just to
> make things simple, but the hardware is I2C. Why must there be a PHY?
> Maybe it is a Base-K link, i.e. a baseboard link to a switch, or a BMC
> or something.

There's no requirement for an MDIO bus or a PHY. If the MAC is connected to
a PHY, only the PHY node needs to be described. The PHY can be controlled
by any interface, I2C, MDIO, or something else. If there's no PHY, the
fixed-link property would be used to describe the link.

> 
>> The only difference is the firmware of the ethernet
>> controller controls... What exactly does the firmware control that a Linux
>> driver would have controlled instead? Just configuring the link settings of
>> the MAC?
> 
> A MAC driver implements struct ethtool_ops:::get_link_settings and
> set_link_settings. For a MAC driver using phylib or phylink they
> typically then call into phylib or phylink to do the actual work,
> maybe with a bit of pre-processing in the MAC driver.
> 
> A MAC driver using firmware would typically make an RPC into the
> firmware to implement these calls.
> 
> There is a MAC driver currently under review which does not have a PHY
> at all. The MAC is directly connected to a switch, all within one
> IC. The link is always running at 5Gbps, the link is always up. It is
> physically impossible to connect a PHY, so get_link_settings just
> returns hard coded values.

The fixed-link property would be used to describe the link of the MAC here.

Arınç

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Andrew Lunn]

> > There is a MAC driver currently under review which does not have a PHY
> > at all. The MAC is directly connected to a switch, all within one
> > IC. The link is always running at 5Gbps, the link is always up. It is
> > physically impossible to connect a PHY, so get_link_settings just
> > returns hard coded values.
> 
> The fixed-link property would be used to describe the link of the MAC here.

Fixed-link make sense for a general purpose MAC which could be
connected to a PHY, or could also be used without a PHY. fixed-link
simplifies the code in that the MAC driver does not see a difference,
it all looks like a PHY.

However for a MAC which cannot be connected to a PHY, there is no need
to emulate a PHY. The MAC driver will be simpler. So i would not
recommend a fixed-link in this situation.

	  Andrew

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

On 24/09/2023 06:15, Andrew Lunn wrote:
>>> There is a MAC driver currently under review which does not have a PHY
>>> at all. The MAC is directly connected to a switch, all within one
>>> IC. The link is always running at 5Gbps, the link is always up. It is
>>> physically impossible to connect a PHY, so get_link_settings just
>>> returns hard coded values.
>>
>> The fixed-link property would be used to describe the link of the MAC here.
> 
> Fixed-link make sense for a general purpose MAC which could be
> connected to a PHY, or could also be used without a PHY. fixed-link
> simplifies the code in that the MAC driver does not see a difference,
> it all looks like a PHY.
> 
> However for a MAC which cannot be connected to a PHY, there is no need
> to emulate a PHY. The MAC driver will be simpler. So i would not
> recommend a fixed-link in this situation.

There's a link, it must be described. The MAC driver can configure the link
without reading the fixed-link property as there's no room for guessing.

The phy-handle, pcs-handle, and sfp properties point out there's a PHY. The
fixed-link property can be used standalone to describe MAC to MAC links.

For this specific ethernet controller, the phy-handle, pcs-handle, and sfp
properties can be disallowed on its schema to point out the ethernet
controller cannot be connected to a PHY.

Arınç

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Andrew Lunn]

On Sun, Sep 24, 2023 at 10:49:49AM +0300, Arınç ÜNAL wrote:
> On 24/09/2023 06:15, Andrew Lunn wrote:
> > > > There is a MAC driver currently under review which does not have a PHY
> > > > at all. The MAC is directly connected to a switch, all within one
> > > > IC. The link is always running at 5Gbps, the link is always up. It is
> > > > physically impossible to connect a PHY, so get_link_settings just
> > > > returns hard coded values.
> > > 
> > > The fixed-link property would be used to describe the link of the MAC here.
> > 
> > Fixed-link make sense for a general purpose MAC which could be
> > connected to a PHY, or could also be used without a PHY. fixed-link
> > simplifies the code in that the MAC driver does not see a difference,
> > it all looks like a PHY.
> > 
> > However for a MAC which cannot be connected to a PHY, there is no need
> > to emulate a PHY. The MAC driver will be simpler. So i would not
> > recommend a fixed-link in this situation.
> 
> There's a link, it must be described.

Why must it be described?

Lets take this to the extreme to make a point. The chip has a ground
pin. Must i describe that?

     Andrew

Re: [PATCH net-next v2 00/10] define and enforce phylink bindings

[Arınç ÜNAL]

On 24.09.2023 17:55, Andrew Lunn wrote:
> On Sun, Sep 24, 2023 at 10:49:49AM +0300, Arınç ÜNAL wrote:
>> On 24/09/2023 06:15, Andrew Lunn wrote:
>>>>> There is a MAC driver currently under review which does not have a PHY
>>>>> at all. The MAC is directly connected to a switch, all within one
>>>>> IC. The link is always running at 5Gbps, the link is always up. It is
>>>>> physically impossible to connect a PHY, so get_link_settings just
>>>>> returns hard coded values.
>>>>
>>>> The fixed-link property would be used to describe the link of the MAC here.
>>>
>>> Fixed-link make sense for a general purpose MAC which could be
>>> connected to a PHY, or could also be used without a PHY. fixed-link
>>> simplifies the code in that the MAC driver does not see a difference,
>>> it all looks like a PHY.
>>>
>>> However for a MAC which cannot be connected to a PHY, there is no need
>>> to emulate a PHY. The MAC driver will be simpler. So i would not
>>> recommend a fixed-link in this situation.
>>
>> There's a link, it must be described.
> 
> Why must it be described?
> 
> Lets take this to the extreme to make a point. The chip has a ground
> pin. Must i describe that?

I think it depends on how important the information is, to be put on the
devicetree. I don't think a ground pin of an SoC is important enough to be
described on the devicetree. It could be described as a text on the
relevant devicetree document though. I've recently submitted a patch that
does a similar thing. I've described which pin groups represent which pins.

https://lore.kernel.org/lkml/20230917162837.277405-2-arinc.unal@arinc9.com/

For an ethernet controller, its link is the core part of the hardware.
Therefore describing the link was deemed important. Hence certain
properties were made to describe the link on the devicetree.

All I proposed was to make sure these properties are always defined on the
devicetree since, for an ethernet controller to exist, it must have a link.

Arınç