Re: [PATCH v2 net-next 4/4] net: dsa: validate that DT nodes of shared ports have the properties they need

[Vladimir Oltean <vladimir.oltean@nxp.com>]

Hi Rob,

On Fri, Jul 29, 2022 at 12:39:06PM -0600, Rob Herring wrote:
> All valid points. At least for the sea of warnings, you can limit
> checking to only a subset of schemas you care about. Setting
> 'DT_SCHEMA_FILES=net/' will only check networking schemas for example.
> Just need folks to care about those subsets.
> 
> I'm not saying don't put warnings in the kernel for this. Just don't
> make it the only source of warnings. Given you are tightening the
> requirements, it makes sense to have a warning. If it was something
> entirely new, then I'd be more inclined to say only the schema should
> check.

How does this look like? It says that if the 'ethernet' property exists
and is a phandle (i.e. this is a CPU port), or if the 'link' property
exists and is a phandle-array (i.e. this is a DSA port), then the
phylink-related properties are mandatory, in the combinations that they
may appear in.

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index 09317e16cb5d..ed828cec90fd 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -76,6 +76,31 @@ properties:
 required:
   - reg
 
+if:
+  oneOf:
+    - properties:
+        ethernet:
+          items:
+            $ref: /schemas/types.yaml#/definitions/phandle
+    - properties:
+        link:
+          items:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+then:
+  allOf:
+  - required:
+    - phy-mode
+  - oneOf:
+    - required:
+      - fixed-link
+    - required:
+      - phy-handle
+    - required:
+      - managed
+    - required:
+      - phy-handle
+      - managed
+
 additionalProperties: true
 

I have deliberately made this part of dsa-port.yaml and not depend on
any compatible string. The reason is the code - we'll warn about missing
properties regardless of whether we have fallback logic for some older
switches. Essentially the fact that some switches have the fallback to
not use phylink will remain an undocumented easter egg as far as the
dt-schema is concerned.

What do you think?

Can I also get an ACK for this patch and patch 1 please?