Re: [PATCH net-next v11 2/9] dt-bindings: net: lan9645x: add LAN9645X switch bindings

This is my reply to:
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260805-dsa_lan9645x_switch_driver_base-v11-0-007ebc983a0a%40microchip.com?part=2

b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml

quoted

new file mode 100644
index 0000000000000..043fb48922b41

--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dsa/microchip,lan96455s-switch.yaml

@@ -0,0 +1,111 @@

[ ... ]

quoted

+  ethernet-ports:
+    type: object
+    patternProperties:
+      "^ethernet-port@[0-8]$":

Discovered by: opus-5 · Missed by: sonnet-5, sashiko-gemini · Confirmed by: sonnet-5
Are the constraints under this pattern actually reachable for all legal
port node names?

The top-level $ref pulls in dsa.yaml#/$defs/ethernet-ports, which accepts:

  patternProperties:
    "^(ethernet-)?ports$":
      patternProperties:
        "^(ethernet-)?port@[0-9a-f]+$":
          description: Ethernet switch ports
          $ref: dsa-port.yaml#
          unevaluatedProperties: false

So both port@N and ethernet-port@N are valid spellings, and indices are
not limited to 0-8. The local ethernet-ports object here has no
additionalProperties or unevaluatedProperties of its own, so children whose
names do not match "^ethernet-port@[0-8]$" are simply not checked against
the local constraints.

That means these two both pass dt_binding_check:

  ethernet-ports {
      port@7 {
          phy-mode = "rgmii";
          rx-internal-delay-ps = <500>;   /* const: 2000 not applied */
      };
  };

  ethernet-ports {
      ethernet-port@9 {                   /* index > 8 not rejected */
          reg = <9>;
      };
  };

Would using the "^(ethernet-)?port@[0-8]$" spelling here, as
Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.yaml does with
"^(ethernet-)?port@6$", make the new per-port constraints effective?

Yes, I will update the regex.

quoted

+        type: object
+        description: Ethernet switch ports
+
+        $ref: dsa-port.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          rx-internal-delay-ps:
+            const: 2000
+
+          tx-internal-delay-ps:
+            const: 2000

Discovered by: opus-5, sashiko-gemini · Missed by: sonnet-5 · Confirmed by: sonnet-5
Should these two properties be guarded by an rgmii phy-mode conditional?

As written they are declared for every port index 0-8 with no phy-mode
condition, and since they live in the same schema object that carries
unevaluatedProperties: false, they count as evaluated for any phy-mode.
This passes validation:

  ethernet-port@0 {
      reg = <0>;
      phy-mode = "gmii";
      rx-internal-delay-ps = <2000>;
  };

Per the driver, only ports 7 and 8 have RGMII pins
(lan9645x_phylink_get_caps(): 0..4 GMII/cuphy, 5..6 SerDes, 7..8 RGMII),
and only lan9645x_rgmii_dll_config() programs the delay lines, with
RGMII_IDX(port) defined for ports 4/7/8 only. On a gmii port the value is
stored in p->rx_internal_delay by lan9645x_port_setup() and then never
used, with no diagnostic.

The existing pattern is a conditional, see
Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.yaml:

  "^(ethernet-)?port@6$":
    allOf:
      - if:
          properties:
            phy-mode:
              contains:
                enum:
                  - rgmii
        then:
          properties:
            rx-internal-delay-ps: ...

Documentation/devicetree/bindings/net/ethernet-controller.yaml also defines
these properties only inside an rgmii* phy-mode conditional.

Yes, I will a conditional here.

Separately, ethernet-controller.yaml states:

  A value of 2000ps, i.e 2ns, and a phy-mode of "rgmii" will not be
  accepted by Reviewers

which is the combination this binding mandates with const: 2000 and shows
in the example below. Is a phy-mode of "rgmii" together with a 2000 ps
internal delay intended here, rather than one of the rgmii-rxid /
rgmii-txid / rgmii-id modes?

I think ethernet-controller.yaml talks about the situation where there is a PHY
present. The intent here is a mac to mac connection. I was specifically
instructed to model it like this, and not do use rgmii-*id as a way to
communicate delay programming of the MAC. It is also clear from
Documentation/networking/phy.rst
that this is the right way to model mac to mac.

quoted

+
+$ref: dsa.yaml#/$defs/ethernet-ports
+
+required:
+  - compatible
+  - reg
+  - ethernet-ports
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    ethernet-switch@4000 {
+        compatible = "microchip,lan96459f-switch", "microchip,lan96455s-switch";
+        reg = <0x4000 0x244>;
+
+        ethernet-ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ethernet-port@0 {
+                reg = <0>;
+                phy-mode = "gmii";
+                phy-handle = <&cuphy0>;
+            };

[ ... ]

quoted

+
+            ethernet-port@7 {
+                reg = <7>;
+                phy-mode = "rgmii";
+                ethernet = <&cpu_host_port>;
+                rx-internal-delay-ps = <2000>;
+                tx-internal-delay-ps = <2000>;
+
+                fixed-link {
+                    speed = <1000>;
+                    full-duplex;
+                    pause;
+                };
+            };
+        };
+    };
+...
+

      ^^^

Discovered by: opus-5 · Missed by: sonnet-5, sashiko-gemini · Confirmed by: sonnet-5
This isn't a bug, but there is a blank line after the end-of-document
marker at the end of the file, which git diff --check reports as "new blank
line at EOF". The other bindings in this directory end with "..." and
nothing after it.

I will remove this.

Cross-instance finding from sashiko-gemini

(008974e58f4042f8866239fc681c104931d4e9cd342a5b5be37f42504a115cf0):

Discovered by: sashiko-gemini, opus-5 · Missed by: sonnet-5 · Confirmed by: opus-5
The schema lacks an `mdio` node definition while setting `unevaluatedProperties: false` at the root,

which inadvertently prevents instantiation of the integrated PHYs referenced in the example.