Re: [PATCH v3 2/7] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

On 17/01/2023 07:52, yanhong wang wrote:

On 2023/1/6 20:44, Krzysztof Kozlowski wrote:

quoted

On 06/01/2023 03:59, Yanhong Wang wrote:

quoted

Some boards(such as StarFive VisionFive v2) require more than one value
which defined by resets property, so the original definition can not
meet the requirements. In order to adapt to different requirements,
adjust the maxitems number definition.

Signed-off-by: Yanhong Wang <redacted>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   | 36 ++++++++++++++-----
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e26c3e76ebb7..f7693e8c8d6d 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

@@ -132,14 +132,6 @@ properties:
         - pclk
         - ptp_ref
 
-  resets:
-    maxItems: 1
-    description:
-      MAC Reset signal.
-
-  reset-names:
-    const: stmmaceth
-
   power-domains:
     maxItems: 1
 

@@ -463,6 +455,34 @@ allOf:
             Enables the TSO feature otherwise it will be managed by
             MAC HW capability register.
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7110-dwmac
+

Looking at your next binding patch, this seems a bit clearer. First of
all, this patch on itself has little sense. It's not usable on its own,
because you need the next one.

Probably the snps,dwmac should be just split into common parts used by
devices. It makes code much less readable and unnecessary complicated to
support in one schema both devices and re-usability.

Otherwise I propose to make the resets/reset-names just like clocks are
made: define here wide constraints and update all other users of this
binding to explicitly restrict resets.

Thanks, refer to the definition of clocks. If it is defined as follows, is it OK?

properties:
  resets:
    minItems: 1
    maxItems: 3
    additionalItems: true

Drop

    items:
      - description: MAC Reset signal.

Drop both

  reset-names:
    minItems: 1
    maxItems: 3
    additionalItems: true

Drop

    contains:
      enum:
        - stmmaceth

Drop all

allOf:
  - if:
      properties:
        compatible:
          contains:
            const: starfive,jh7110-dwmac
    then:
      properties:
        resets:
          minItems: 2
          maxItems: 2
        reset-names:
          items:
            - const: stmmaceth
            - const: ahb
      required:
        - resets
        - reset-names  
    else:
      properties:
        resets:
          maxItems: 1
          description:
            MAC Reset signal.

        reset-names:
          const: stmmaceth

Do you have any other better suggestions?

More or less like this but the allOf should not be in snps,dwmac schema
but in individual schemas. The snps,dwmac is growing unmaintainable...

Best regards,
Krzysztof