[PATCH RFC v1 0/4] dwmac-meson8b: picosecond precision RX delay support

[PATCH RFC v1 0/4] dwmac-meson8b: picosecond precision RX delay support

[Martin Blumenstingl]

Hello,

with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).

The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.

While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.

I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
	&ext_mdio {
		external_phy: ethernet-phy@0 {
			/* Realtek RTL8211F (0x001cc916) */
			reg = <0>;
			eee-broken-1000t;

			reset-assert-us = <10000>;
			reset-deassert-us = <30000>;
			reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
						GPIO_OPEN_DRAIN)>;

			interrupt-parent = <&gpio_intc>;
			/* MAC_INTR on GPIOZ_14 */
			interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
		};
	};

	&ethmac {
		status = "okay";

		pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>;
		pinctrl-names = "default";

		phy-mode = "rgmii-txid";
		phy-handle = <&external_phy>;

		amlogic,rgmii-rx-delay-ps = <800>;
	};


[0] https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=KyWnsjY3JA@mail.gmail.com/



Martin Blumenstingl (4):
  dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
  net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A

 .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++-
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 92 +++++++++++++++----
 2 files changed, 123 insertions(+), 21 deletions(-)

-- 
2.29.2

[PATCH RFC v1 0/4] dwmac-meson8b: picosecond precision RX delay support

[Martin Blumenstingl]

Hello,

with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).

The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.

While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.

I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
	&ext_mdio {
		external_phy: ethernet-phy@0 {
			/* Realtek RTL8211F (0x001cc916) */
			reg = <0>;
			eee-broken-1000t;

			reset-assert-us = <10000>;
			reset-deassert-us = <30000>;
			reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
						GPIO_OPEN_DRAIN)>;

			interrupt-parent = <&gpio_intc>;
			/* MAC_INTR on GPIOZ_14 */
			interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
		};
	};

	&ethmac {
		status = "okay";

		pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>;
		pinctrl-names = "default";

		phy-mode = "rgmii-txid";
		phy-handle = <&external_phy>;

		amlogic,rgmii-rx-delay-ps = <800>;
	};


[0] https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=KyWnsjY3JA@mail.gmail.com/



Martin Blumenstingl (4):
  dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
  net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A

 .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++-
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 92 +++++++++++++++----
 2 files changed, 123 insertions(+), 21 deletions(-)

-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v1 0/4] dwmac-meson8b: picosecond precision RX delay support

[Martin Blumenstingl]

Hello,

with the help of Jianxin Pan (many thanks!) the meaning of the "new"
PRG_ETH1[19:16] register bits on Amlogic Meson G12A, G12B and SM1 SoCs
are finally known. These SoCs allow fine-tuning the RGMII RX delay in
200ps steps (contrary to what I have thought in the past [0] these are
not some "calibration" values).

The vendor u-boot has code to automatically detect the best RX/TX delay
settings. For now we keep it simple and add a device-tree property with
200ps precision to select the "right" RX delay for each board.

While here, deprecate the "amlogic,rx-delay-ns" property as it's not
used on any upstream .dts (yet). The driver is backwards compatible.

I have tested this on an X96 Air 4GB board (not upstream yet). Testing
with iperf3 gives 938 Mbits/sec in both directions (RX and TX). The
following network settings were used in the .dts (2ns TX delay
generated by the PHY, 800ps RX delay generated by the MAC as the PHY
only supports 0ns or 2ns RX delays):
	&ext_mdio {
		external_phy: ethernet-phy@0 {
			/* Realtek RTL8211F (0x001cc916) */
			reg = <0>;
			eee-broken-1000t;

			reset-assert-us = <10000>;
			reset-deassert-us = <30000>;
			reset-gpios = <&gpio GPIOZ_15 (GPIO_ACTIVE_LOW |
						GPIO_OPEN_DRAIN)>;

			interrupt-parent = <&gpio_intc>;
			/* MAC_INTR on GPIOZ_14 */
			interrupts = <26 IRQ_TYPE_LEVEL_LOW>;
		};
	};

	&ethmac {
		status = "okay";

		pinctrl-0 = <&eth_pins>, <&eth_rgmii_pins>;
		pinctrl-names = "default";

		phy-mode = "rgmii-txid";
		phy-handle = <&external_phy>;

		amlogic,rgmii-rx-delay-ps = <800>;
	};


[0] https://lore.kernel.org/netdev/CAFBinCATt4Hi9rigj52nMf3oygyFbnopZcsakGL=KyWnsjY3JA@mail.gmail.com/



Martin Blumenstingl (4):
  dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay
  net: stmmac: dwmac-meson8b: move RGMII delays into a separate function
  net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A

 .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++-
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 92 +++++++++++++++----
 2 files changed, 123 insertions(+), 21 deletions(-)

-- 
2.29.2


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Deprecate the old
"amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
property.

For older SoCs the only known supported values were 0ns and 2ns. The new
SoCs have 200ps precision and support RGMII RX delays between 0ps and
3000ps.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
index 6b057b117aa0..bafde1194193 100644
--- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -74,18 +74,68 @@ allOf:
             Any configuration is ignored when the phy-mode is set to "rmii".
 
         amlogic,rx-delay-ns:
+          deprecated: true
           enum:
             - 0
             - 2
           default: 0
+          description:
+            The internal RGMII RX clock delay in nanoseconds. Deprecated, use
+            amlogic,rgmii-rx-delay-ps instead.
+
+        amlogic,rgmii-rx-delay-ps:
+          default: 0
           description:
             The internal RGMII RX clock delay (provided by this IP block) in
-            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
+            picoseconds. When phy-mode is set to "rgmii" then the RX delay
             should be explicitly configured. When the phy-mode is set to
             either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
             provided by the PHY. Any configuration is ignored when the
             phy-mode is set to "rmii".
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson8b-dwmac
+              - amlogic,meson8m2-dwmac
+              - amlogic,meson-gxbb-dwmac
+              - amlogic,meson-axg-dwmac
+    then:
+      properties:
+        amlogic,rgmii-rx-delay-ps:
+          enum:
+            - 0
+            - 2000
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson-g12a-dwmac
+    then:
+      properties:
+        amlogic,rgmii-rx-delay-ps:
+          enum:
+            - 0
+            - 200
+            - 400
+            - 600
+            - 800
+            - 1000
+            - 1200
+            - 1400
+            - 1600
+            - 1800
+            - 2000
+            - 2200
+            - 2400
+            - 2600
+            - 2800
+            - 3000
+
 properties:
   compatible:
     additionalItems: true
-- 
2.29.2

[PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Deprecate the old
"amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
property.

For older SoCs the only known supported values were 0ns and 2ns. The new
SoCs have 200ps precision and support RGMII RX delays between 0ps and
3000ps.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
index 6b057b117aa0..bafde1194193 100644
--- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -74,18 +74,68 @@ allOf:
             Any configuration is ignored when the phy-mode is set to "rmii".
 
         amlogic,rx-delay-ns:
+          deprecated: true
           enum:
             - 0
             - 2
           default: 0
+          description:
+            The internal RGMII RX clock delay in nanoseconds. Deprecated, use
+            amlogic,rgmii-rx-delay-ps instead.
+
+        amlogic,rgmii-rx-delay-ps:
+          default: 0
           description:
             The internal RGMII RX clock delay (provided by this IP block) in
-            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
+            picoseconds. When phy-mode is set to "rgmii" then the RX delay
             should be explicitly configured. When the phy-mode is set to
             either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
             provided by the PHY. Any configuration is ignored when the
             phy-mode is set to "rmii".
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson8b-dwmac
+              - amlogic,meson8m2-dwmac
+              - amlogic,meson-gxbb-dwmac
+              - amlogic,meson-axg-dwmac
+    then:
+      properties:
+        amlogic,rgmii-rx-delay-ps:
+          enum:
+            - 0
+            - 2000
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson-g12a-dwmac
+    then:
+      properties:
+        amlogic,rgmii-rx-delay-ps:
+          enum:
+            - 0
+            - 200
+            - 400
+            - 600
+            - 800
+            - 1000
+            - 1200
+            - 1400
+            - 1600
+            - 1800
+            - 2000
+            - 2200
+            - 2400
+            - 2600
+            - 2800
+            - 3000
+
 properties:
   compatible:
     additionalItems: true
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Deprecate the old
"amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
property.

For older SoCs the only known supported values were 0ns and 2ns. The new
SoCs have 200ps precision and support RGMII RX delays between 0ps and
3000ps.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
index 6b057b117aa0..bafde1194193 100644
--- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
@@ -74,18 +74,68 @@ allOf:
             Any configuration is ignored when the phy-mode is set to "rmii".
 
         amlogic,rx-delay-ns:
+          deprecated: true
           enum:
             - 0
             - 2
           default: 0
+          description:
+            The internal RGMII RX clock delay in nanoseconds. Deprecated, use
+            amlogic,rgmii-rx-delay-ps instead.
+
+        amlogic,rgmii-rx-delay-ps:
+          default: 0
           description:
             The internal RGMII RX clock delay (provided by this IP block) in
-            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
+            picoseconds. When phy-mode is set to "rgmii" then the RX delay
             should be explicitly configured. When the phy-mode is set to
             either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
             provided by the PHY. Any configuration is ignored when the
             phy-mode is set to "rmii".
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson8b-dwmac
+              - amlogic,meson8m2-dwmac
+              - amlogic,meson-gxbb-dwmac
+              - amlogic,meson-axg-dwmac
+    then:
+      properties:
+        amlogic,rgmii-rx-delay-ps:
+          enum:
+            - 0
+            - 2000
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - amlogic,meson-g12a-dwmac
+    then:
+      properties:
+        amlogic,rgmii-rx-delay-ps:
+          enum:
+            - 0
+            - 200
+            - 400
+            - 600
+            - 800
+            - 1000
+            - 1200
+            - 1400
+            - 1600
+            - 1800
+            - 2000
+            - 2200
+            - 2400
+            - 2600
+            - 2800
+            - 3000
+
 properties:
   compatible:
     additionalItems: true
-- 
2.29.2


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH RFC v1 2/4] net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Parse the new
"amlogic,rgmii-rx-delay-ps" property or fall back to the old
"amlogic,rx-delay-ns".

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index dc0b8b6d180d..465eaf000da1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -83,7 +83,7 @@ struct meson8b_dwmac {
 	phy_interface_t			phy_mode;
 	struct clk			*rgmii_tx_clk;
 	u32				tx_delay_ns;
-	u32				rx_delay_ns;
+	u32				rx_delay_ps;
 	struct clk			*timing_adj_clk;
 };
 
@@ -276,7 +276,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 	tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
 				   dwmac->tx_delay_ns >> 1);
 
-	if (dwmac->rx_delay_ns == 2)
+	if (dwmac->rx_delay_ps == 2000)
 		rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
 	else
 		rx_dly_config = 0;
@@ -406,14 +406,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 				 &dwmac->tx_delay_ns))
 		dwmac->tx_delay_ns = 2;
 
-	/* use 0ns as fallback since this is what most boards actually use */
-	if (of_property_read_u32(pdev->dev.of_node, "amlogic,rx-delay-ns",
-				 &dwmac->rx_delay_ns))
-		dwmac->rx_delay_ns = 0;
+	/* RX delay defaults to 0ps since this is what many boards use */
+	if (of_property_read_u32(pdev->dev.of_node,
+				 "amlogic,rgmii-rx-delay-ps",
+				  &dwmac->rx_delay_ps)) {
+		if (!of_property_read_u32(pdev->dev.of_node,
+					  "amlogic,rx-delay-ns",
+					  &dwmac->rx_delay_ps))
+			/* convert ns to ps */
+			dwmac->rx_delay_ps *= 1000;
+	}
 
-	if (dwmac->rx_delay_ns != 0 && dwmac->rx_delay_ns != 2) {
+	if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
 		dev_err(&pdev->dev,
-			"The only allowed RX delays values are: 0ns, 2ns");
+			"The only allowed RX delays values are: 0ps, 2000ps");
 		ret = -EINVAL;
 		goto err_remove_config_dt;
 	}
-- 
2.29.2

[PATCH RFC v1 2/4] net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Parse the new
"amlogic,rgmii-rx-delay-ps" property or fall back to the old
"amlogic,rx-delay-ns".

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index dc0b8b6d180d..465eaf000da1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -83,7 +83,7 @@ struct meson8b_dwmac {
 	phy_interface_t			phy_mode;
 	struct clk			*rgmii_tx_clk;
 	u32				tx_delay_ns;
-	u32				rx_delay_ns;
+	u32				rx_delay_ps;
 	struct clk			*timing_adj_clk;
 };
 
@@ -276,7 +276,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 	tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
 				   dwmac->tx_delay_ns >> 1);
 
-	if (dwmac->rx_delay_ns == 2)
+	if (dwmac->rx_delay_ps == 2000)
 		rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
 	else
 		rx_dly_config = 0;
@@ -406,14 +406,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 				 &dwmac->tx_delay_ns))
 		dwmac->tx_delay_ns = 2;
 
-	/* use 0ns as fallback since this is what most boards actually use */
-	if (of_property_read_u32(pdev->dev.of_node, "amlogic,rx-delay-ns",
-				 &dwmac->rx_delay_ns))
-		dwmac->rx_delay_ns = 0;
+	/* RX delay defaults to 0ps since this is what many boards use */
+	if (of_property_read_u32(pdev->dev.of_node,
+				 "amlogic,rgmii-rx-delay-ps",
+				  &dwmac->rx_delay_ps)) {
+		if (!of_property_read_u32(pdev->dev.of_node,
+					  "amlogic,rx-delay-ns",
+					  &dwmac->rx_delay_ps))
+			/* convert ns to ps */
+			dwmac->rx_delay_ps *= 1000;
+	}
 
-	if (dwmac->rx_delay_ns != 0 && dwmac->rx_delay_ns != 2) {
+	if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
 		dev_err(&pdev->dev,
-			"The only allowed RX delays values are: 0ns, 2ns");
+			"The only allowed RX delays values are: 0ps, 2000ps");
 		ret = -EINVAL;
 		goto err_remove_config_dt;
 	}
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v1 2/4] net: stmmac: dwmac-meson8b: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
delay register which allows picoseconds precision. Parse the new
"amlogic,rgmii-rx-delay-ps" property or fall back to the old
"amlogic,rx-delay-ns".

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index dc0b8b6d180d..465eaf000da1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -83,7 +83,7 @@ struct meson8b_dwmac {
 	phy_interface_t			phy_mode;
 	struct clk			*rgmii_tx_clk;
 	u32				tx_delay_ns;
-	u32				rx_delay_ns;
+	u32				rx_delay_ps;
 	struct clk			*timing_adj_clk;
 };
 
@@ -276,7 +276,7 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 	tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
 				   dwmac->tx_delay_ns >> 1);
 
-	if (dwmac->rx_delay_ns == 2)
+	if (dwmac->rx_delay_ps == 2000)
 		rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
 	else
 		rx_dly_config = 0;
@@ -406,14 +406,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 				 &dwmac->tx_delay_ns))
 		dwmac->tx_delay_ns = 2;
 
-	/* use 0ns as fallback since this is what most boards actually use */
-	if (of_property_read_u32(pdev->dev.of_node, "amlogic,rx-delay-ns",
-				 &dwmac->rx_delay_ns))
-		dwmac->rx_delay_ns = 0;
+	/* RX delay defaults to 0ps since this is what many boards use */
+	if (of_property_read_u32(pdev->dev.of_node,
+				 "amlogic,rgmii-rx-delay-ps",
+				  &dwmac->rx_delay_ps)) {
+		if (!of_property_read_u32(pdev->dev.of_node,
+					  "amlogic,rx-delay-ns",
+					  &dwmac->rx_delay_ps))
+			/* convert ns to ps */
+			dwmac->rx_delay_ps *= 1000;
+	}
 
-	if (dwmac->rx_delay_ns != 0 && dwmac->rx_delay_ns != 2) {
+	if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
 		dev_err(&pdev->dev,
-			"The only allowed RX delays values are: 0ns, 2ns");
+			"The only allowed RX delays values are: 0ps, 2000ps");
 		ret = -EINVAL;
 		goto err_remove_config_dt;
 	}
-- 
2.29.2


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH RFC v1 3/4] net: stmmac: dwmac-meson8b: move RGMII delays into a separate function

[Martin Blumenstingl]

Newer SoCs starting with the Amlogic Meson G12A have more a precise
RGMII RX delay configuration register. This means more complexity in the
code. Extract the existing RGMII delay configuration code into a
separate function to make it easier to read/understand even when adding
more logic in the future.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 465eaf000da1..a6e2077ed871 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -268,7 +268,7 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
 	return 0;
 }
 
-static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 {
 	u32 tx_dly_config, rx_dly_config, delay_config;
 	int ret;
@@ -323,6 +323,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 				PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
 				delay_config);
 
+	return 0;
+}
+
+static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+{
+	int ret;
+
 	if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
 		/* only relevant for RMII mode -> disable in RGMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
@@ -431,6 +438,10 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 		goto err_remove_config_dt;
 	}
 
+	ret = meson8b_init_rgmii_delays(dwmac);
+	if (ret)
+		goto err_remove_config_dt;
+
 	ret = meson8b_init_rgmii_tx_clk(dwmac);
 	if (ret)
 		goto err_remove_config_dt;
-- 
2.29.2

[PATCH RFC v1 3/4] net: stmmac: dwmac-meson8b: move RGMII delays into a separate function

[Martin Blumenstingl]

Newer SoCs starting with the Amlogic Meson G12A have more a precise
RGMII RX delay configuration register. This means more complexity in the
code. Extract the existing RGMII delay configuration code into a
separate function to make it easier to read/understand even when adding
more logic in the future.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 465eaf000da1..a6e2077ed871 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -268,7 +268,7 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
 	return 0;
 }
 
-static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 {
 	u32 tx_dly_config, rx_dly_config, delay_config;
 	int ret;
@@ -323,6 +323,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 				PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
 				delay_config);
 
+	return 0;
+}
+
+static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+{
+	int ret;
+
 	if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
 		/* only relevant for RMII mode -> disable in RGMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
@@ -431,6 +438,10 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 		goto err_remove_config_dt;
 	}
 
+	ret = meson8b_init_rgmii_delays(dwmac);
+	if (ret)
+		goto err_remove_config_dt;
+
 	ret = meson8b_init_rgmii_tx_clk(dwmac);
 	if (ret)
 		goto err_remove_config_dt;
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v1 3/4] net: stmmac: dwmac-meson8b: move RGMII delays into a separate function

[Martin Blumenstingl]

Newer SoCs starting with the Amlogic Meson G12A have more a precise
RGMII RX delay configuration register. This means more complexity in the
code. Extract the existing RGMII delay configuration code into a
separate function to make it easier to read/understand even when adding
more logic in the future.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index 465eaf000da1..a6e2077ed871 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -268,7 +268,7 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
 	return 0;
 }
 
-static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 {
 	u32 tx_dly_config, rx_dly_config, delay_config;
 	int ret;
@@ -323,6 +323,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
 				PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
 				delay_config);
 
+	return 0;
+}
+
+static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
+{
+	int ret;
+
 	if (phy_interface_mode_is_rgmii(dwmac->phy_mode)) {
 		/* only relevant for RMII mode -> disable in RGMII mode */
 		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
@@ -431,6 +438,10 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 		goto err_remove_config_dt;
 	}
 
+	ret = meson8b_init_rgmii_delays(dwmac);
+	if (ret)
+		goto err_remove_config_dt;
+
 	ret = meson8b_init_rgmii_tx_clk(dwmac);
 	if (ret)
 		goto err_remove_config_dt;
-- 
2.29.2


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH RFC v1 4/4] net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A

[Martin Blumenstingl]

Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
delay logic. Instead of fine-tuning the delay in the nanoseconds range
it now allows tuning in 200 picosecond steps. This support comes with
new bits in the PRG_ETH1[19:16] register.

Add support for validating the RGMII RX delay as well as configuring the
register accordingly on these platforms.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 63 ++++++++++++++-----
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index a6e2077ed871..ae081fbc0552 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -69,10 +69,21 @@
  */
 #define PRG_ETH0_ADJ_SKEW		GENMASK(24, 20)
 
+#define PRG_ETH1			0x4
+
+/* Defined for adding a delay to the input RX_CLK for better timing.
+ * Each step is 200ps. These bits are used with external RGMII PHYs
+ * because RGMII RX only has the small window. cfg_rxclk_dly can
+ * adjust the window between RX_CLK and RX_DATA and improve the stability
+ * of "rx data valid".
+ */
+#define PRG_ETH1_CFG_RXCLK_DLY		GENMASK(19, 16)
+
 struct meson8b_dwmac;
 
 struct meson8b_dwmac_data {
 	int (*set_phy_mode)(struct meson8b_dwmac *dwmac);
+	bool has_prg_eth1_rgmii_rx_delay;
 };
 
 struct meson8b_dwmac {
@@ -270,30 +281,35 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
 
 static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 {
-	u32 tx_dly_config, rx_dly_config, delay_config;
+	u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config;
 	int ret;
 
+	rx_adj_config = 0;
+	cfg_rxclk_dly = 0;
 	tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
 				   dwmac->tx_delay_ns >> 1);
 
-	if (dwmac->rx_delay_ps == 2000)
-		rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
-	else
-		rx_dly_config = 0;
+	if (dwmac->data->has_prg_eth1_rgmii_rx_delay)
+		cfg_rxclk_dly = FIELD_PREP(PRG_ETH1_CFG_RXCLK_DLY,
+					   dwmac->rx_delay_ps / 200);
+	else if (dwmac->rx_delay_ps == 2000)
+		rx_adj_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
-		delay_config = tx_dly_config | rx_dly_config;
+		delay_config = tx_dly_config | rx_adj_config;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 		delay_config = tx_dly_config;
+		cfg_rxclk_dly = 0;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		delay_config = rx_dly_config;
+		delay_config = rx_adj_config;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RMII:
 		delay_config = 0;
+		cfg_rxclk_dly = 0;
 		break;
 	default:
 		dev_err(dwmac->dev, "unsupported phy-mode %s\n",
@@ -301,7 +317,7 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 		return -EINVAL;
 	}
 
-	if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+	if (rx_adj_config & PRG_ETH0_ADJ_ENABLE) {
 		if (!dwmac->timing_adj_clk) {
 			dev_err(dwmac->dev,
 				"The timing-adjustment clock is mandatory for the RX delay re-timing\n");
@@ -323,6 +339,9 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 				PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
 				delay_config);
 
+	meson8b_dwmac_mask_bits(dwmac, PRG_ETH1, PRG_ETH1_CFG_RXCLK_DLY,
+				cfg_rxclk_dly);
+
 	return 0;
 }
 
@@ -424,11 +443,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 			dwmac->rx_delay_ps *= 1000;
 	}
 
-	if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
-		dev_err(&pdev->dev,
-			"The only allowed RX delays values are: 0ps, 2000ps");
-		ret = -EINVAL;
-		goto err_remove_config_dt;
+	if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
+		if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+			dev_err(dwmac->dev,
+				"The only allowed RGMII RX delays values are: 0ps, 2000ps");
+			ret = -EINVAL;
+			goto err_remove_config_dt;
+		}
+	} else {
+		if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+			dev_err(dwmac->dev,
+				"The RGMII RX delay range is 0..3000ps in 200ps steps");
+			ret = -EINVAL;
+			goto err_remove_config_dt;
+		}
 	}
 
 	dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
@@ -470,10 +498,17 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 
 static const struct meson8b_dwmac_data meson8b_dwmac_data = {
 	.set_phy_mode = meson8b_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = false,
 };
 
 static const struct meson8b_dwmac_data meson_axg_dwmac_data = {
 	.set_phy_mode = meson_axg_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = false,
+};
+
+static const struct meson8b_dwmac_data meson_g12a_dwmac_data = {
+	.set_phy_mode = meson_axg_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = true,
 };
 
 static const struct of_device_id meson8b_dwmac_match[] = {
@@ -495,7 +530,7 @@ static const struct of_device_id meson8b_dwmac_match[] = {
 	},
 	{
 		.compatible = "amlogic,meson-g12a-dwmac",
-		.data = &meson_axg_dwmac_data,
+		.data = &meson_g12a_dwmac_data,
 	},
 	{ }
 };
-- 
2.29.2

[PATCH RFC v1 4/4] net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A

[Martin Blumenstingl]

Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
delay logic. Instead of fine-tuning the delay in the nanoseconds range
it now allows tuning in 200 picosecond steps. This support comes with
new bits in the PRG_ETH1[19:16] register.

Add support for validating the RGMII RX delay as well as configuring the
register accordingly on these platforms.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 63 ++++++++++++++-----
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index a6e2077ed871..ae081fbc0552 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -69,10 +69,21 @@
  */
 #define PRG_ETH0_ADJ_SKEW		GENMASK(24, 20)
 
+#define PRG_ETH1			0x4
+
+/* Defined for adding a delay to the input RX_CLK for better timing.
+ * Each step is 200ps. These bits are used with external RGMII PHYs
+ * because RGMII RX only has the small window. cfg_rxclk_dly can
+ * adjust the window between RX_CLK and RX_DATA and improve the stability
+ * of "rx data valid".
+ */
+#define PRG_ETH1_CFG_RXCLK_DLY		GENMASK(19, 16)
+
 struct meson8b_dwmac;
 
 struct meson8b_dwmac_data {
 	int (*set_phy_mode)(struct meson8b_dwmac *dwmac);
+	bool has_prg_eth1_rgmii_rx_delay;
 };
 
 struct meson8b_dwmac {
@@ -270,30 +281,35 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
 
 static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 {
-	u32 tx_dly_config, rx_dly_config, delay_config;
+	u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config;
 	int ret;
 
+	rx_adj_config = 0;
+	cfg_rxclk_dly = 0;
 	tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
 				   dwmac->tx_delay_ns >> 1);
 
-	if (dwmac->rx_delay_ps == 2000)
-		rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
-	else
-		rx_dly_config = 0;
+	if (dwmac->data->has_prg_eth1_rgmii_rx_delay)
+		cfg_rxclk_dly = FIELD_PREP(PRG_ETH1_CFG_RXCLK_DLY,
+					   dwmac->rx_delay_ps / 200);
+	else if (dwmac->rx_delay_ps == 2000)
+		rx_adj_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
-		delay_config = tx_dly_config | rx_dly_config;
+		delay_config = tx_dly_config | rx_adj_config;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 		delay_config = tx_dly_config;
+		cfg_rxclk_dly = 0;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		delay_config = rx_dly_config;
+		delay_config = rx_adj_config;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RMII:
 		delay_config = 0;
+		cfg_rxclk_dly = 0;
 		break;
 	default:
 		dev_err(dwmac->dev, "unsupported phy-mode %s\n",
@@ -301,7 +317,7 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 		return -EINVAL;
 	}
 
-	if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+	if (rx_adj_config & PRG_ETH0_ADJ_ENABLE) {
 		if (!dwmac->timing_adj_clk) {
 			dev_err(dwmac->dev,
 				"The timing-adjustment clock is mandatory for the RX delay re-timing\n");
@@ -323,6 +339,9 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 				PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
 				delay_config);
 
+	meson8b_dwmac_mask_bits(dwmac, PRG_ETH1, PRG_ETH1_CFG_RXCLK_DLY,
+				cfg_rxclk_dly);
+
 	return 0;
 }
 
@@ -424,11 +443,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 			dwmac->rx_delay_ps *= 1000;
 	}
 
-	if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
-		dev_err(&pdev->dev,
-			"The only allowed RX delays values are: 0ps, 2000ps");
-		ret = -EINVAL;
-		goto err_remove_config_dt;
+	if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
+		if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+			dev_err(dwmac->dev,
+				"The only allowed RGMII RX delays values are: 0ps, 2000ps");
+			ret = -EINVAL;
+			goto err_remove_config_dt;
+		}
+	} else {
+		if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+			dev_err(dwmac->dev,
+				"The RGMII RX delay range is 0..3000ps in 200ps steps");
+			ret = -EINVAL;
+			goto err_remove_config_dt;
+		}
 	}
 
 	dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
@@ -470,10 +498,17 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 
 static const struct meson8b_dwmac_data meson8b_dwmac_data = {
 	.set_phy_mode = meson8b_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = false,
 };
 
 static const struct meson8b_dwmac_data meson_axg_dwmac_data = {
 	.set_phy_mode = meson_axg_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = false,
+};
+
+static const struct meson8b_dwmac_data meson_g12a_dwmac_data = {
+	.set_phy_mode = meson_axg_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = true,
 };
 
 static const struct of_device_id meson8b_dwmac_match[] = {
@@ -495,7 +530,7 @@ static const struct of_device_id meson8b_dwmac_match[] = {
 	},
 	{
 		.compatible = "amlogic,meson-g12a-dwmac",
-		.data = &meson_axg_dwmac_data,
+		.data = &meson_g12a_dwmac_data,
 	},
 	{ }
 };
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH RFC v1 4/4] net: stmmac: dwmac-meson8b: add support for the RGMII RX delay on G12A

[Martin Blumenstingl]

Amlogic Meson G12A (and newer: G12B, SM1) SoCs have a more advanced RX
delay logic. Instead of fine-tuning the delay in the nanoseconds range
it now allows tuning in 200 picosecond steps. This support comes with
new bits in the PRG_ETH1[19:16] register.

Add support for validating the RGMII RX delay as well as configuring the
register accordingly on these platforms.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 63 ++++++++++++++-----
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index a6e2077ed871..ae081fbc0552 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -69,10 +69,21 @@
  */
 #define PRG_ETH0_ADJ_SKEW		GENMASK(24, 20)
 
+#define PRG_ETH1			0x4
+
+/* Defined for adding a delay to the input RX_CLK for better timing.
+ * Each step is 200ps. These bits are used with external RGMII PHYs
+ * because RGMII RX only has the small window. cfg_rxclk_dly can
+ * adjust the window between RX_CLK and RX_DATA and improve the stability
+ * of "rx data valid".
+ */
+#define PRG_ETH1_CFG_RXCLK_DLY		GENMASK(19, 16)
+
 struct meson8b_dwmac;
 
 struct meson8b_dwmac_data {
 	int (*set_phy_mode)(struct meson8b_dwmac *dwmac);
+	bool has_prg_eth1_rgmii_rx_delay;
 };
 
 struct meson8b_dwmac {
@@ -270,30 +281,35 @@ static int meson8b_devm_clk_prepare_enable(struct meson8b_dwmac *dwmac,
 
 static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 {
-	u32 tx_dly_config, rx_dly_config, delay_config;
+	u32 tx_dly_config, rx_adj_config, cfg_rxclk_dly, delay_config;
 	int ret;
 
+	rx_adj_config = 0;
+	cfg_rxclk_dly = 0;
 	tx_dly_config = FIELD_PREP(PRG_ETH0_TXDLY_MASK,
 				   dwmac->tx_delay_ns >> 1);
 
-	if (dwmac->rx_delay_ps == 2000)
-		rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
-	else
-		rx_dly_config = 0;
+	if (dwmac->data->has_prg_eth1_rgmii_rx_delay)
+		cfg_rxclk_dly = FIELD_PREP(PRG_ETH1_CFG_RXCLK_DLY,
+					   dwmac->rx_delay_ps / 200);
+	else if (dwmac->rx_delay_ps == 2000)
+		rx_adj_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
 
 	switch (dwmac->phy_mode) {
 	case PHY_INTERFACE_MODE_RGMII:
-		delay_config = tx_dly_config | rx_dly_config;
+		delay_config = tx_dly_config | rx_adj_config;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 		delay_config = tx_dly_config;
+		cfg_rxclk_dly = 0;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		delay_config = rx_dly_config;
+		delay_config = rx_adj_config;
 		break;
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RMII:
 		delay_config = 0;
+		cfg_rxclk_dly = 0;
 		break;
 	default:
 		dev_err(dwmac->dev, "unsupported phy-mode %s\n",
@@ -301,7 +317,7 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 		return -EINVAL;
 	}
 
-	if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
+	if (rx_adj_config & PRG_ETH0_ADJ_ENABLE) {
 		if (!dwmac->timing_adj_clk) {
 			dev_err(dwmac->dev,
 				"The timing-adjustment clock is mandatory for the RX delay re-timing\n");
@@ -323,6 +339,9 @@ static int meson8b_init_rgmii_delays(struct meson8b_dwmac *dwmac)
 				PRG_ETH0_ADJ_DELAY | PRG_ETH0_ADJ_SKEW,
 				delay_config);
 
+	meson8b_dwmac_mask_bits(dwmac, PRG_ETH1, PRG_ETH1_CFG_RXCLK_DLY,
+				cfg_rxclk_dly);
+
 	return 0;
 }
 
@@ -424,11 +443,20 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 			dwmac->rx_delay_ps *= 1000;
 	}
 
-	if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
-		dev_err(&pdev->dev,
-			"The only allowed RX delays values are: 0ps, 2000ps");
-		ret = -EINVAL;
-		goto err_remove_config_dt;
+	if (dwmac->data->has_prg_eth1_rgmii_rx_delay) {
+		if (dwmac->rx_delay_ps != 0 && dwmac->rx_delay_ps != 2000) {
+			dev_err(dwmac->dev,
+				"The only allowed RGMII RX delays values are: 0ps, 2000ps");
+			ret = -EINVAL;
+			goto err_remove_config_dt;
+		}
+	} else {
+		if (dwmac->rx_delay_ps > 3000 || dwmac->rx_delay_ps % 200) {
+			dev_err(dwmac->dev,
+				"The RGMII RX delay range is 0..3000ps in 200ps steps");
+			ret = -EINVAL;
+			goto err_remove_config_dt;
+		}
 	}
 
 	dwmac->timing_adj_clk = devm_clk_get_optional(dwmac->dev,
@@ -470,10 +498,17 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
 
 static const struct meson8b_dwmac_data meson8b_dwmac_data = {
 	.set_phy_mode = meson8b_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = false,
 };
 
 static const struct meson8b_dwmac_data meson_axg_dwmac_data = {
 	.set_phy_mode = meson_axg_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = false,
+};
+
+static const struct meson8b_dwmac_data meson_g12a_dwmac_data = {
+	.set_phy_mode = meson_axg_set_phy_mode,
+	.has_prg_eth1_rgmii_rx_delay = true,
 };
 
 static const struct of_device_id meson8b_dwmac_match[] = {
@@ -495,7 +530,7 @@ static const struct of_device_id meson8b_dwmac_match[] = {
 	},
 	{
 		.compatible = "amlogic,meson-g12a-dwmac",
-		.data = &meson_axg_dwmac_data,
+		.data = &meson_g12a_dwmac_data,
 	},
 	{ }
 };
-- 
2.29.2


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Andrew Lunn]

On Sat, Nov 14, 2020 at 09:01:01PM +0100, Martin Blumenstingl wrote:
> Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> delay register which allows picoseconds precision. Deprecate the old
> "amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
> property.
> 
> For older SoCs the only known supported values were 0ns and 2ns. The new
> SoCs have 200ps precision and support RGMII RX delays between 0ps and
> 3000ps.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index 6b057b117aa0..bafde1194193 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> @@ -74,18 +74,68 @@ allOf:
>              Any configuration is ignored when the phy-mode is set to "rmii".
>  
>          amlogic,rx-delay-ns:
> +          deprecated: true
>            enum:
>              - 0
>              - 2
>            default: 0
> +          description:
> +            The internal RGMII RX clock delay in nanoseconds. Deprecated, use
> +            amlogic,rgmii-rx-delay-ps instead.
> +
> +        amlogic,rgmii-rx-delay-ps:
> +          default: 0
>            description:
>              The internal RGMII RX clock delay (provided by this IP block) in
> -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
>              should be explicitly configured. When the phy-mode is set to
>              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
>              provided by the PHY. Any configuration is ignored when the
>              phy-mode is set to "rmii".

Hi Martin

I don't think the wording matches what the driver is actually doing:

        if (dwmac->rx_delay_ns == 2)
                rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
        else
                rx_dly_config = 0;

        switch (dwmac->phy_mode) {
        case PHY_INTERFACE_MODE_RGMII:
                delay_config = tx_dly_config | rx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_RXID:
                delay_config = tx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_TXID:
                delay_config = rx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_ID:
        case PHY_INTERFACE_MODE_RMII:
                delay_config = 0;
                break;

So rx_delay is used for both rgmii and rgmii-txid. The binding says
nothing about rgmii-txid.

And while i'm looking at the code, i wonder about this:

       if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
                if (!dwmac->timing_adj_clk) {
                        dev_err(dwmac->dev,
                                "The timing-adjustment clock is mandatory for the RX delay re-timing\n");
                        return -EINVAL;
                }

                /* The timing adjustment logic is driven by a separate clock */
                ret = meson8b_devm_clk_prepare_enable(dwmac,
                                                      dwmac->timing_adj_clk);
                if (ret) {
                        dev_err(dwmac->dev,
                                "Failed to enable the timing-adjustment clock\n");
                        return ret;
                }
        }

It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
true, so the clock is enabled, but delay_config does not contain
rx_delay_config, so it is pointless turning it on.

	Andrew

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Andrew Lunn]

On Sat, Nov 14, 2020 at 09:01:01PM +0100, Martin Blumenstingl wrote:
> Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> delay register which allows picoseconds precision. Deprecate the old
> "amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
> property.
> 
> For older SoCs the only known supported values were 0ns and 2ns. The new
> SoCs have 200ps precision and support RGMII RX delays between 0ps and
> 3000ps.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index 6b057b117aa0..bafde1194193 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> @@ -74,18 +74,68 @@ allOf:
>              Any configuration is ignored when the phy-mode is set to "rmii".
>  
>          amlogic,rx-delay-ns:
> +          deprecated: true
>            enum:
>              - 0
>              - 2
>            default: 0
> +          description:
> +            The internal RGMII RX clock delay in nanoseconds. Deprecated, use
> +            amlogic,rgmii-rx-delay-ps instead.
> +
> +        amlogic,rgmii-rx-delay-ps:
> +          default: 0
>            description:
>              The internal RGMII RX clock delay (provided by this IP block) in
> -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
>              should be explicitly configured. When the phy-mode is set to
>              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
>              provided by the PHY. Any configuration is ignored when the
>              phy-mode is set to "rmii".

Hi Martin

I don't think the wording matches what the driver is actually doing:

        if (dwmac->rx_delay_ns == 2)
                rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
        else
                rx_dly_config = 0;

        switch (dwmac->phy_mode) {
        case PHY_INTERFACE_MODE_RGMII:
                delay_config = tx_dly_config | rx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_RXID:
                delay_config = tx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_TXID:
                delay_config = rx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_ID:
        case PHY_INTERFACE_MODE_RMII:
                delay_config = 0;
                break;

So rx_delay is used for both rgmii and rgmii-txid. The binding says
nothing about rgmii-txid.

And while i'm looking at the code, i wonder about this:

       if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
                if (!dwmac->timing_adj_clk) {
                        dev_err(dwmac->dev,
                                "The timing-adjustment clock is mandatory for the RX delay re-timing\n");
                        return -EINVAL;
                }

                /* The timing adjustment logic is driven by a separate clock */
                ret = meson8b_devm_clk_prepare_enable(dwmac,
                                                      dwmac->timing_adj_clk);
                if (ret) {
                        dev_err(dwmac->dev,
                                "Failed to enable the timing-adjustment clock\n");
                        return ret;
                }
        }

It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
true, so the clock is enabled, but delay_config does not contain
rx_delay_config, so it is pointless turning it on.

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Andrew Lunn]

On Sat, Nov 14, 2020 at 09:01:01PM +0100, Martin Blumenstingl wrote:
> Amlogic Meson G12A, G12B and SM1 SoCs have a more advanced RGMII RX
> delay register which allows picoseconds precision. Deprecate the old
> "amlogic,rx-delay-ns" in favour of a new "amlogic,rgmii-rx-delay-ps"
> property.
> 
> For older SoCs the only known supported values were 0ns and 2ns. The new
> SoCs have 200ps precision and support RGMII RX delays between 0ps and
> 3000ps.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  .../bindings/net/amlogic,meson-dwmac.yaml     | 52 ++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> index 6b057b117aa0..bafde1194193 100644
> --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> @@ -74,18 +74,68 @@ allOf:
>              Any configuration is ignored when the phy-mode is set to "rmii".
>  
>          amlogic,rx-delay-ns:
> +          deprecated: true
>            enum:
>              - 0
>              - 2
>            default: 0
> +          description:
> +            The internal RGMII RX clock delay in nanoseconds. Deprecated, use
> +            amlogic,rgmii-rx-delay-ps instead.
> +
> +        amlogic,rgmii-rx-delay-ps:
> +          default: 0
>            description:
>              The internal RGMII RX clock delay (provided by this IP block) in
> -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
>              should be explicitly configured. When the phy-mode is set to
>              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
>              provided by the PHY. Any configuration is ignored when the
>              phy-mode is set to "rmii".

Hi Martin

I don't think the wording matches what the driver is actually doing:

        if (dwmac->rx_delay_ns == 2)
                rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
        else
                rx_dly_config = 0;

        switch (dwmac->phy_mode) {
        case PHY_INTERFACE_MODE_RGMII:
                delay_config = tx_dly_config | rx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_RXID:
                delay_config = tx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_TXID:
                delay_config = rx_dly_config;
                break;
        case PHY_INTERFACE_MODE_RGMII_ID:
        case PHY_INTERFACE_MODE_RMII:
                delay_config = 0;
                break;

So rx_delay is used for both rgmii and rgmii-txid. The binding says
nothing about rgmii-txid.

And while i'm looking at the code, i wonder about this:

       if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
                if (!dwmac->timing_adj_clk) {
                        dev_err(dwmac->dev,
                                "The timing-adjustment clock is mandatory for the RX delay re-timing\n");
                        return -EINVAL;
                }

                /* The timing adjustment logic is driven by a separate clock */
                ret = meson8b_devm_clk_prepare_enable(dwmac,
                                                      dwmac->timing_adj_clk);
                if (ret) {
                        dev_err(dwmac->dev,
                                "Failed to enable the timing-adjustment clock\n");
                        return ret;
                }
        }

It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
true, so the clock is enabled, but delay_config does not contain
rx_delay_config, so it is pointless turning it on.

	Andrew

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Hi Andrew,

On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
[...]
> > +        amlogic,rgmii-rx-delay-ps:
> > +          default: 0
> >            description:
> >              The internal RGMII RX clock delay (provided by this IP block) in
> > -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> > +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
> >              should be explicitly configured. When the phy-mode is set to
> >              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
> >              provided by the PHY. Any configuration is ignored when the
> >              phy-mode is set to "rmii".
>
> Hi Martin
>
> I don't think the wording matches what the driver is actually doing:
>
>         if (dwmac->rx_delay_ns == 2)
>                 rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
>         else
>                 rx_dly_config = 0;
>
>         switch (dwmac->phy_mode) {
>         case PHY_INTERFACE_MODE_RGMII:
>                 delay_config = tx_dly_config | rx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_RXID:
>                 delay_config = tx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_TXID:
>                 delay_config = rx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_ID:
>         case PHY_INTERFACE_MODE_RMII:
>                 delay_config = 0;
>                 break;
>
> So rx_delay is used for both rgmii and rgmii-txid. The binding says
> nothing about rgmii-txid.
interesting point here. it's been like this before this patch. still I
would like to understand what the proper way to fix it is so I can
also include a fix for it:
1. should rgmii-txid not add any RX delay on the MAC side? that would
mean for my board I will switch to phy-mode rgmii so the MAC applies
both the RX and TX delays
2. update the documentation to clarify that rgmii-txid would also add
the RX delay on the MAC side

> And while i'm looking at the code, i wonder about this:
>
>        if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
>                 if (!dwmac->timing_adj_clk) {
>                         dev_err(dwmac->dev,
>                                 "The timing-adjustment clock is mandatory for the RX delay re-timing\n");
>                         return -EINVAL;
>                 }
>
>                 /* The timing adjustment logic is driven by a separate clock */
>                 ret = meson8b_devm_clk_prepare_enable(dwmac,
>                                                       dwmac->timing_adj_clk);
>                 if (ret) {
>                         dev_err(dwmac->dev,
>                                 "Failed to enable the timing-adjustment clock\n");
>                         return ret;
>                 }
>         }
>
> It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
> true, so the clock is enabled, but delay_config does not contain
> rx_delay_config, so it is pointless turning it on.
that is a good point and also a bug with one of the previous patches
I'll include a patch fixing this in v2


Best regards,
Martin

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Hi Andrew,

On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
[...]
> > +        amlogic,rgmii-rx-delay-ps:
> > +          default: 0
> >            description:
> >              The internal RGMII RX clock delay (provided by this IP block) in
> > -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> > +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
> >              should be explicitly configured. When the phy-mode is set to
> >              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
> >              provided by the PHY. Any configuration is ignored when the
> >              phy-mode is set to "rmii".
>
> Hi Martin
>
> I don't think the wording matches what the driver is actually doing:
>
>         if (dwmac->rx_delay_ns == 2)
>                 rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
>         else
>                 rx_dly_config = 0;
>
>         switch (dwmac->phy_mode) {
>         case PHY_INTERFACE_MODE_RGMII:
>                 delay_config = tx_dly_config | rx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_RXID:
>                 delay_config = tx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_TXID:
>                 delay_config = rx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_ID:
>         case PHY_INTERFACE_MODE_RMII:
>                 delay_config = 0;
>                 break;
>
> So rx_delay is used for both rgmii and rgmii-txid. The binding says
> nothing about rgmii-txid.
interesting point here. it's been like this before this patch. still I
would like to understand what the proper way to fix it is so I can
also include a fix for it:
1. should rgmii-txid not add any RX delay on the MAC side? that would
mean for my board I will switch to phy-mode rgmii so the MAC applies
both the RX and TX delays
2. update the documentation to clarify that rgmii-txid would also add
the RX delay on the MAC side

> And while i'm looking at the code, i wonder about this:
>
>        if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
>                 if (!dwmac->timing_adj_clk) {
>                         dev_err(dwmac->dev,
>                                 "The timing-adjustment clock is mandatory for the RX delay re-timing\n");
>                         return -EINVAL;
>                 }
>
>                 /* The timing adjustment logic is driven by a separate clock */
>                 ret = meson8b_devm_clk_prepare_enable(dwmac,
>                                                       dwmac->timing_adj_clk);
>                 if (ret) {
>                         dev_err(dwmac->dev,
>                                 "Failed to enable the timing-adjustment clock\n");
>                         return ret;
>                 }
>         }
>
> It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
> true, so the clock is enabled, but delay_config does not contain
> rx_delay_config, so it is pointless turning it on.
that is a good point and also a bug with one of the previous patches
I'll include a patch fixing this in v2


Best regards,
Martin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Martin Blumenstingl]

Hi Andrew,

On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
[...]
> > +        amlogic,rgmii-rx-delay-ps:
> > +          default: 0
> >            description:
> >              The internal RGMII RX clock delay (provided by this IP block) in
> > -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> > +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
> >              should be explicitly configured. When the phy-mode is set to
> >              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
> >              provided by the PHY. Any configuration is ignored when the
> >              phy-mode is set to "rmii".
>
> Hi Martin
>
> I don't think the wording matches what the driver is actually doing:
>
>         if (dwmac->rx_delay_ns == 2)
>                 rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
>         else
>                 rx_dly_config = 0;
>
>         switch (dwmac->phy_mode) {
>         case PHY_INTERFACE_MODE_RGMII:
>                 delay_config = tx_dly_config | rx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_RXID:
>                 delay_config = tx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_TXID:
>                 delay_config = rx_dly_config;
>                 break;
>         case PHY_INTERFACE_MODE_RGMII_ID:
>         case PHY_INTERFACE_MODE_RMII:
>                 delay_config = 0;
>                 break;
>
> So rx_delay is used for both rgmii and rgmii-txid. The binding says
> nothing about rgmii-txid.
interesting point here. it's been like this before this patch. still I
would like to understand what the proper way to fix it is so I can
also include a fix for it:
1. should rgmii-txid not add any RX delay on the MAC side? that would
mean for my board I will switch to phy-mode rgmii so the MAC applies
both the RX and TX delays
2. update the documentation to clarify that rgmii-txid would also add
the RX delay on the MAC side

> And while i'm looking at the code, i wonder about this:
>
>        if (rx_dly_config & PRG_ETH0_ADJ_ENABLE) {
>                 if (!dwmac->timing_adj_clk) {
>                         dev_err(dwmac->dev,
>                                 "The timing-adjustment clock is mandatory for the RX delay re-timing\n");
>                         return -EINVAL;
>                 }
>
>                 /* The timing adjustment logic is driven by a separate clock */
>                 ret = meson8b_devm_clk_prepare_enable(dwmac,
>                                                       dwmac->timing_adj_clk);
>                 if (ret) {
>                         dev_err(dwmac->dev,
>                                 "Failed to enable the timing-adjustment clock\n");
>                         return ret;
>                 }
>         }
>
> It looks like it can be that rx_dly_config & PRG_ETH0_ADJ_ENABLE is
> true, so the clock is enabled, but delay_config does not contain
> rx_delay_config, so it is pointless turning it on.
that is a good point and also a bug with one of the previous patches
I'll include a patch fixing this in v2


Best regards,
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Andrew Lunn]

On Sun, Nov 15, 2020 at 10:22:06AM +0100, Martin Blumenstingl wrote:
> Hi Andrew,
> 
> On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
> [...]
> > > +        amlogic,rgmii-rx-delay-ps:
> > > +          default: 0
> > >            description:
> > >              The internal RGMII RX clock delay (provided by this IP block) in
> > > -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> > > +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
> > >              should be explicitly configured. When the phy-mode is set to
> > >              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
> > >              provided by the PHY. Any configuration is ignored when the
> > >              phy-mode is set to "rmii".
> >
> > Hi Martin
> >
> > I don't think the wording matches what the driver is actually doing:
> >
> >         if (dwmac->rx_delay_ns == 2)
> >                 rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
> >         else
> >                 rx_dly_config = 0;
> >
> >         switch (dwmac->phy_mode) {
> >         case PHY_INTERFACE_MODE_RGMII:
> >                 delay_config = tx_dly_config | rx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_RXID:
> >                 delay_config = tx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_TXID:
> >                 delay_config = rx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_ID:
> >         case PHY_INTERFACE_MODE_RMII:
> >                 delay_config = 0;
> >                 break;
> >
> > So rx_delay is used for both rgmii and rgmii-txid. The binding says
> > nothing about rgmii-txid.
> interesting point here. it's been like this before this patch. still I
> would like to understand what the proper way to fix it is so I can
> also include a fix for it:
> 1. should rgmii-txid not add any RX delay on the MAC side? that would
> mean for my board I will switch to phy-mode rgmii so the MAC applies
> both the RX and TX delays
> 2. update the documentation to clarify that rgmii-txid would also add
> the RX delay on the MAC side

Hi Martin

I would fix the documentation.

> that is a good point and also a bug with one of the previous patches
> I'll include a patch fixing this in v2

Thanks for looking at these.

       Andrew

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Andrew Lunn]

On Sun, Nov 15, 2020 at 10:22:06AM +0100, Martin Blumenstingl wrote:
> Hi Andrew,
> 
> On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
> [...]
> > > +        amlogic,rgmii-rx-delay-ps:
> > > +          default: 0
> > >            description:
> > >              The internal RGMII RX clock delay (provided by this IP block) in
> > > -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> > > +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
> > >              should be explicitly configured. When the phy-mode is set to
> > >              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
> > >              provided by the PHY. Any configuration is ignored when the
> > >              phy-mode is set to "rmii".
> >
> > Hi Martin
> >
> > I don't think the wording matches what the driver is actually doing:
> >
> >         if (dwmac->rx_delay_ns == 2)
> >                 rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
> >         else
> >                 rx_dly_config = 0;
> >
> >         switch (dwmac->phy_mode) {
> >         case PHY_INTERFACE_MODE_RGMII:
> >                 delay_config = tx_dly_config | rx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_RXID:
> >                 delay_config = tx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_TXID:
> >                 delay_config = rx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_ID:
> >         case PHY_INTERFACE_MODE_RMII:
> >                 delay_config = 0;
> >                 break;
> >
> > So rx_delay is used for both rgmii and rgmii-txid. The binding says
> > nothing about rgmii-txid.
> interesting point here. it's been like this before this patch. still I
> would like to understand what the proper way to fix it is so I can
> also include a fix for it:
> 1. should rgmii-txid not add any RX delay on the MAC side? that would
> mean for my board I will switch to phy-mode rgmii so the MAC applies
> both the RX and TX delays
> 2. update the documentation to clarify that rgmii-txid would also add
> the RX delay on the MAC side

Hi Martin

I would fix the documentation.

> that is a good point and also a bug with one of the previous patches
> I'll include a patch fixing this in v2

Thanks for looking at these.

       Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH RFC v1 1/4] dt-bindings: net: dwmac-meson: use picoseconds for the RGMII RX delay

[Andrew Lunn]

On Sun, Nov 15, 2020 at 10:22:06AM +0100, Martin Blumenstingl wrote:
> Hi Andrew,
> 
> On Sat, Nov 14, 2020 at 11:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
> [...]
> > > +        amlogic,rgmii-rx-delay-ps:
> > > +          default: 0
> > >            description:
> > >              The internal RGMII RX clock delay (provided by this IP block) in
> > > -            nanoseconds. When phy-mode is set to "rgmii" then the RX delay
> > > +            picoseconds. When phy-mode is set to "rgmii" then the RX delay
> > >              should be explicitly configured. When the phy-mode is set to
> > >              either "rgmii-id" or "rgmii-rxid" the RX clock delay is already
> > >              provided by the PHY. Any configuration is ignored when the
> > >              phy-mode is set to "rmii".
> >
> > Hi Martin
> >
> > I don't think the wording matches what the driver is actually doing:
> >
> >         if (dwmac->rx_delay_ns == 2)
> >                 rx_dly_config = PRG_ETH0_ADJ_ENABLE | PRG_ETH0_ADJ_SETUP;
> >         else
> >                 rx_dly_config = 0;
> >
> >         switch (dwmac->phy_mode) {
> >         case PHY_INTERFACE_MODE_RGMII:
> >                 delay_config = tx_dly_config | rx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_RXID:
> >                 delay_config = tx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_TXID:
> >                 delay_config = rx_dly_config;
> >                 break;
> >         case PHY_INTERFACE_MODE_RGMII_ID:
> >         case PHY_INTERFACE_MODE_RMII:
> >                 delay_config = 0;
> >                 break;
> >
> > So rx_delay is used for both rgmii and rgmii-txid. The binding says
> > nothing about rgmii-txid.
> interesting point here. it's been like this before this patch. still I
> would like to understand what the proper way to fix it is so I can
> also include a fix for it:
> 1. should rgmii-txid not add any RX delay on the MAC side? that would
> mean for my board I will switch to phy-mode rgmii so the MAC applies
> both the RX and TX delays
> 2. update the documentation to clarify that rgmii-txid would also add
> the RX delay on the MAC side

Hi Martin

I would fix the documentation.

> that is a good point and also a bug with one of the previous patches
> I'll include a patch fixing this in v2

Thanks for looking at these.

       Andrew

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic