Hi Andrew, 

Sorry for the delay in response.
Starting now, I will be taking over this task.
I have gone through your comments and feedback and will be implementing them
in v4 of this patch.

-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch]
Sent: 15 August 2023 02:10
To: Sriranjani P <redacted>
Cc: 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; 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

quoted

+  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.

What clock are you skew-ing here? And why?

As per customer's requirement, we need 2ns delay in fsys block both in TX
and RX path.

quoted

+    ethernet_1: ethernet@14300000 {
+              compatible = "tesla,dwc-qos-ethernet-4.21";
+              reg = <0x0 0x14300000 0x0 0x10000>;
+              interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+              clocks = <&clock_peric

PERIC_EQOS_TOP_IPCLKPORT_CLK_PTP_REF_I>,

quoted

+                       <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_ACLK_I>,
+                       <&clock_peric PERIC_EQOS_TOP_IPCLKPORT_HCLK_I>,
+                       <&clock_peric

PERIC_EQOS_TOP_IPCLKPORT_RGMII_CLK_I>,

quoted

+                       <&clock_peric

PERIC_EQOS_TOP_IPCLKPORT_CLK_RX_I>,

quoted

+                       <&clock_peric

PERIC_BUS_D_PERIC_IPCLKPORT_EQOSCLK>,

quoted

+                       <&clock_peric

PERIC_BUS_P_PERIC_IPCLKPORT_EQOSCLK>,

quoted

+                       <&clock_peric PERIC_EQOS_PHYRXCLK_MUX>,
+                       <&clock_peric PERIC_EQOS_PHYRXCLK>,
+                       <&clock_peric PERIC_DOUT_RGMII_CLK>;
+              clock-names = "ptp_ref",
+                            "master_bus",
+                            "slave_bus",
+                            "tx",
+                            "rx",
+                            "master2_bus",
+                            "slave2_bus",
+                            "eqos_rxclk_mux",
+                            "eqos_phyrxclk",
+                            "dout_peric_rgmii_clk";
+              pinctrl-names = "default";
+              pinctrl-0 = <&eth1_tx_clk>, <&eth1_tx_data>,

<&eth1_tx_ctrl>,

quoted

+                          <&eth1_phy_intr>, <&eth1_rx_clk>,

<&eth1_rx_data>,

quoted

+                          <&eth1_rx_ctrl>, <&eth1_mdio>;
+              fsd-rx-clock-skew = <&sysreg_peric 0x10>;
+              iommus = <&smmu_peric 0x0 0x1>;
+              phy-mode = "rgmii";

I know it is just an example, but "rgmii" is generally wrong. "rgmii-id"

is

generally what you need. And when i do see "rgmii", it starts ringing

alarm

bells for me, it could mean your RGMII delays are being handled wrongly.

Thanks for bringing this to our notice. Will correct this in v4 as rgmii-id.

       Andrew

Regards,
Swathi