-----Original Message-----
From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org]
Sent: 15 August 2023 01:10
To: Sriranjani P <redacted>; davem@davemloft.net;
edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
conor+dt@kernel.org; richardcochran@gmail.com;
alexandre.torgue@foss.st.com; joabreu@synopsys.com;
mcoquelin.stm32@gmail.com; alim.akhtar@samsung.com; linux-
fsd@tesla.com; pankaj.dubey@samsung.com; swathi.ks@samsung.com;
ravi.patel@samsung.com
Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm-
kernel@lists.infradead.org
Subject: Re: [PATCH v3 1/4] dt-bindings: net: Add FSD EQoS device tree
bindings

On 14/08/2023 13:25, Sriranjani P wrote:

quoted

Add FSD Ethernet compatible in Synopsys dt-bindings document. Add FSD
Ethernet YAML schema to enable the DT validation.

Signed-off-by: Pankaj Dubey <redacted>
Signed-off-by: Ravi Patel <redacted>
Signed-off-by: Swathi K S <redacted>
Signed-off-by: Sriranjani P <redacted>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   |   5 +-
 .../devicetree/bindings/net/tesla,ethqos.yaml | 114
++++++++++++++++++
 2 files changed, 117 insertions(+), 2 deletions(-)  create mode
100644 Documentation/devicetree/bindings/net/tesla,ethqos.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml

b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index ddf9522a5dc2..0ced7901e644 100644

--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml

@@ -96,6 +96,7 @@ properties:
         - snps,dwxgmac
         - snps,dwxgmac-2.10
         - starfive,jh7110-dwmac
+        - tesla,fsd-ethqos-4.21

I don't think one given SoC - and I was told FSD is strictly defined one specific
SoC - can have different versions of the same block, so drop the block
versioning.

Ok, will remove versioning.

quoted

   reg:
     minItems: 1

@@ -117,7 +118,7 @@ properties:

   clocks:
     minItems: 1
-    maxItems: 8
+    maxItems: 10
     additionalItems: true
     items:
       - description: GMAC main clock

@@ -129,7 +130,7 @@ properties:

   clock-names:
     minItems: 1
-    maxItems: 8
+    maxItems: 10
     additionalItems: true
     contains:
       enum:

diff --git a/Documentation/devicetree/bindings/net/tesla,ethqos.yaml

b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml
new file mode 100644
index 000000000000..b78829246364

--- /dev/null
+++ b/Documentation/devicetree/bindings/net/tesla,ethqos.yaml

@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
+---
+$id:
+https://protect2.fireeye.com/v1/url?k=0d5f9085-6cd485b3-0d5e1bca-

74fe

quoted

+485cbff1-9835d59b137d73e5&q=1&e=93f28da2-6d86-4cc2-a07a-

9be1380616cc&

quoted

+u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fnet%2Ftesla%2Cethqos.
yaml%2

quoted

+3
+$schema:
+https://protect2.fireeye.com/v1/url?k=67d3522a-0658471c-67d2d965-

74fe

quoted

+485cbff1-b9570dbbedf33f81&q=1&e=93f28da2-6d86-4cc2-a07a-

9be1380616cc&

quoted

+u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
+
+title: FSD Ethernet Quality of Service
+
+maintainers:
+  - Sriranjani P [off-list ref]
+  - Swathi K S [off-list ref]
+
+description:
+  dwmmac based tesla ethernet devices which support Gigabit
+  ethernet.
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+properties:
+  compatible:
+    const: tesla,fsd-ethqos-4.21.yaml

?

Will fix this to tesla,fsd-ethqos.yaml 

quoted

+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 5

Why? I expect it to be specific.

Sorry, I could not understood this comment. In FSD we have two instances of EQoS IP, one in PERIC block, which requires total 10 clocks  to be configured and another instance exist in FSYS0 block which needs 5 clocks to be configured, so we kept minItems as 5 and maxItems as 10, but looks like latest items schema do not need maxItems entry so we will drop maxItems entry. In my understanding minItems still required so it should be kept with minimum number of clock requirements.

quoted

+    maxItems: 10
+
+  clock-names:
+    minItems: 5
+    maxItems: 10
+    items:
+      - const: ptp_ref
+      - const: master_bus
+      - const: slave_bus
+      - const: tx
+      - const: rx
+      - const: master2_bus
+      - const: slave2_bus
+      - const: eqos_rxclk_mux
+      - const: eqos_phyrxclk
+      - const: dout_peric_rgmii_clk
+
+  fsd-rx-clock-skew:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to the syscon node
+          - description: offset of the control register
+    description:
+      Should be phandle/offset pair. The phandle to the syscon node.
+
+  iommus:
+    maxItems: 1
+
+  phy-mode:
+    $ref: ethernet-controller.yaml#/properties/phy-connection-type
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - rx-clock-skew

Eee? Isn't it fsd-rx-clock-skew which anyway is not correct?

Sorry, I missed to change this in DT schema before posting, I will make this to fsd-rx-clock-skew. 

quoted

+  - iommus
+  - phy-mode
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/fsd-clk.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ethernet_1: ethernet@14300000 {
+              compatible = "tesla,dwc-qos-ethernet-4.21";

Three different compatibles for the same.

No, please test your patches before sending.

I am not even checking if previous feedback was applied... Did you really go
through it?

Sorry, I missed this will take care. 

Best regards,
Krzysztof