[PATCH v5 0/3] sun8i: r40: second ethernet support

[PATCH v5 0/3] sun8i: r40: second ethernet support

[Evgeny Boger]

Maxime, Corentin, thank you for the review of v4!

This patch series adds support for two Ethernet ports on Allwinner R40.

R40 (aka V40,A40i,T3) has two different Ethernet IPs called EMAC and GMAC.
EMAC only support 10/100 Mbit in MII mode, while GMAC support both 10/100
(MII) and 10/100/1000 (RGMII).

In contrast to A10/A20 where GMAC and EMAC share the same pins making EMAC
somewhat pointless, on R40 EMAC can be routed to port H.
Both EMAC (on port H) and GMAC (on port A) can be then enabled at the same 
time, allowing for two ethernet ports.

Tested on custom A40i board with two IP101GRI PHYs in MII mode.

Changes in v5:
 - Rebase on fresh linux-next
 - Remove reset line from example bindings

Changes in v4:
 - Rename sun4i-r40-emac to sun8i-r40-emac to match R40 family.
 - remove redundant status = "okay" in dts
 - less obscure commit messages

Changes in v3:
Minor fixes in bindings. Rebased on top of linux-next/master.
dt_bindings_check and dtbs_check passed.

 - bindings: separate commit for DT bindings
 - bindings: simplify handling of compatible strings
 - bindings: make resets property required on R40
 - dts: get rid of duplicate node

Changes in v2:
 - EMAC reset is no longer optional on R40
 - Add a new DT compatible string for R40 EMAC
 - Deassert reset line before enabling the clock
 - minor fixes: formatting, DT node order, leftover pinctrl props

Evgeny Boger (3):
  net: allwinner: reset control support
  dt-bindings: net: support for Allwinner R40 EMAC controller
  ARM: dts: sun8i: r40: add second ethernet support

 .../net/allwinner,sun4i-a10-emac.yaml         | 17 ++++-
 arch/arm/boot/dts/sun8i-r40.dtsi              | 49 ++++++++++++++
 drivers/net/ethernet/allwinner/sun4i-emac.c   | 64 +++++++++++++++++--
 3 files changed, 124 insertions(+), 6 deletions(-)

-- 
2.25.1

[PATCH v5 0/3] sun8i: r40: second ethernet support

[Evgeny Boger]

Maxime, Corentin, thank you for the review of v4!

This patch series adds support for two Ethernet ports on Allwinner R40.

R40 (aka V40,A40i,T3) has two different Ethernet IPs called EMAC and GMAC.
EMAC only support 10/100 Mbit in MII mode, while GMAC support both 10/100
(MII) and 10/100/1000 (RGMII).

In contrast to A10/A20 where GMAC and EMAC share the same pins making EMAC
somewhat pointless, on R40 EMAC can be routed to port H.
Both EMAC (on port H) and GMAC (on port A) can be then enabled at the same 
time, allowing for two ethernet ports.

Tested on custom A40i board with two IP101GRI PHYs in MII mode.

Changes in v5:
 - Rebase on fresh linux-next
 - Remove reset line from example bindings

Changes in v4:
 - Rename sun4i-r40-emac to sun8i-r40-emac to match R40 family.
 - remove redundant status = "okay" in dts
 - less obscure commit messages

Changes in v3:
Minor fixes in bindings. Rebased on top of linux-next/master.
dt_bindings_check and dtbs_check passed.

 - bindings: separate commit for DT bindings
 - bindings: simplify handling of compatible strings
 - bindings: make resets property required on R40
 - dts: get rid of duplicate node

Changes in v2:
 - EMAC reset is no longer optional on R40
 - Add a new DT compatible string for R40 EMAC
 - Deassert reset line before enabling the clock
 - minor fixes: formatting, DT node order, leftover pinctrl props

Evgeny Boger (3):
  net: allwinner: reset control support
  dt-bindings: net: support for Allwinner R40 EMAC controller
  ARM: dts: sun8i: r40: add second ethernet support

 .../net/allwinner,sun4i-a10-emac.yaml         | 17 ++++-
 arch/arm/boot/dts/sun8i-r40.dtsi              | 49 ++++++++++++++
 drivers/net/ethernet/allwinner/sun4i-emac.c   | 64 +++++++++++++++++--
 3 files changed, 124 insertions(+), 6 deletions(-)

-- 
2.25.1

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

[PATCH v5 1/3] net: allwinner: reset control support

[Evgeny Boger]

R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
However, on R40 the EMAC reset needs to be deasserted.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 drivers/net/ethernet/allwinner/sun4i-emac.c | 64 +++++++++++++++++++--
 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 74635a6fa8ca..56e811b0a279 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -28,6 +28,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
+#include <linux/reset.h>
 #include <linux/soc/sunxi/sunxi_sram.h>
 #include <linux/dmaengine.h>
 
@@ -69,6 +70,15 @@ MODULE_PARM_DESC(watchdog, "transmit timeout in milliseconds");
  * devices, EMACA and EMACB.
  */
 
+/**
+ * struct emac_quirks - Differences between SoC variants.
+ *
+ * @has_reset: SoC needs reset deasserted.
+ */
+struct emac_quirks {
+	bool		has_reset;
+};
+
 struct emac_board_info {
 	struct clk		*clk;
 	struct device		*dev;
@@ -85,6 +95,7 @@ struct emac_board_info {
 	unsigned int		link;
 	unsigned int		speed;
 	unsigned int		duplex;
+	struct reset_control	*reset;
 
 	phy_interface_t		phy_interface;
 	struct dma_chan	*rx_chan;
@@ -968,6 +979,7 @@ static int emac_probe(struct platform_device *pdev)
 	struct emac_board_info *db;
 	struct net_device *ndev;
 	int ret = 0;
+	const struct emac_quirks *quirks;
 
 	ndev = alloc_etherdev(sizeof(struct emac_board_info));
 	if (!ndev) {
@@ -986,6 +998,13 @@ static int emac_probe(struct platform_device *pdev)
 
 	spin_lock_init(&db->lock);
 
+	quirks = of_device_get_match_data(&pdev->dev);
+	if (!quirks) {
+		dev_err(&pdev->dev, "Failed to determine the quirks to use\n");
+		ret = -ENODEV;
+		goto out;
+	}
+
 	db->membase = of_iomap(np, 0);
 	if (!db->membase) {
 		dev_err(&pdev->dev, "failed to remap registers\n");
@@ -1002,19 +1021,34 @@ static int emac_probe(struct platform_device *pdev)
 		goto out_iounmap;
 	}
 
+	if (quirks->has_reset) {
+		db->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+		if (IS_ERR(db->reset)) {
+			dev_err(&pdev->dev, "unable to request reset\n");
+			ret = PTR_ERR(db->reset);
+			goto out_dispose_mapping;
+		}
+
+		ret = reset_control_deassert(db->reset);
+		if (ret) {
+			dev_err(&pdev->dev, "could not deassert EMAC reset\n");
+			goto out_dispose_mapping;
+		}
+	}
+
 	if (emac_configure_dma(db))
 		netdev_info(ndev, "configure dma failed. disable dma.\n");
 
 	db->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(db->clk)) {
 		ret = PTR_ERR(db->clk);
-		goto out_dispose_mapping;
+		goto out_assert_reset;
 	}
 
 	ret = clk_prepare_enable(db->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "Error couldn't enable clock (%d)\n", ret);
-		goto out_dispose_mapping;
+		goto out_assert_reset;
 	}
 
 	ret = sunxi_sram_claim(&pdev->dev);
@@ -1070,6 +1104,8 @@ static int emac_probe(struct platform_device *pdev)
 	sunxi_sram_release(&pdev->dev);
 out_clk_disable_unprepare:
 	clk_disable_unprepare(db->clk);
+out_assert_reset:
+	reset_control_assert(db->reset);
 out_dispose_mapping:
 	irq_dispose_mapping(ndev->irq);
 out_iounmap:
@@ -1095,6 +1131,7 @@ static int emac_remove(struct platform_device *pdev)
 	unregister_netdev(ndev);
 	sunxi_sram_release(&pdev->dev);
 	clk_disable_unprepare(db->clk);
+	reset_control_assert(db->reset);
 	irq_dispose_mapping(ndev->irq);
 	iounmap(db->membase);
 	free_netdev(ndev);
@@ -1126,11 +1163,28 @@ static int emac_resume(struct platform_device *dev)
 	return 0;
 }
 
-static const struct of_device_id emac_of_match[] = {
-	{.compatible = "allwinner,sun4i-a10-emac",},
+static const struct emac_quirks sun4i_a10_emac_quirks = {
+	.has_reset = false,
+};
+
+static const struct emac_quirks sun8i_r40_emac_quirks = {
+	.has_reset = true,
+};
 
+static const struct of_device_id emac_of_match[] = {
+	{
+		.compatible = "allwinner,sun4i-a10-emac",
+		.data = &sun4i_a10_emac_quirks
+	},
+	{
+		.compatible = "allwinner,sun8i-r40-emac",
+		.data = &sun8i_r40_emac_quirks
+	},
 	/* Deprecated */
-	{.compatible = "allwinner,sun4i-emac",},
+	{
+		.compatible = "allwinner,sun4i-emac",
+		.data = &sun4i_a10_emac_quirks
+	},
 	{},
 };
 
-- 
2.25.1

[PATCH v5 1/3] net: allwinner: reset control support

[Evgeny Boger]

R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
However, on R40 the EMAC reset needs to be deasserted.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 drivers/net/ethernet/allwinner/sun4i-emac.c | 64 +++++++++++++++++++--
 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
index 74635a6fa8ca..56e811b0a279 100644
--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
@@ -28,6 +28,7 @@
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/phy.h>
+#include <linux/reset.h>
 #include <linux/soc/sunxi/sunxi_sram.h>
 #include <linux/dmaengine.h>
 
@@ -69,6 +70,15 @@ MODULE_PARM_DESC(watchdog, "transmit timeout in milliseconds");
  * devices, EMACA and EMACB.
  */
 
+/**
+ * struct emac_quirks - Differences between SoC variants.
+ *
+ * @has_reset: SoC needs reset deasserted.
+ */
+struct emac_quirks {
+	bool		has_reset;
+};
+
 struct emac_board_info {
 	struct clk		*clk;
 	struct device		*dev;
@@ -85,6 +95,7 @@ struct emac_board_info {
 	unsigned int		link;
 	unsigned int		speed;
 	unsigned int		duplex;
+	struct reset_control	*reset;
 
 	phy_interface_t		phy_interface;
 	struct dma_chan	*rx_chan;
@@ -968,6 +979,7 @@ static int emac_probe(struct platform_device *pdev)
 	struct emac_board_info *db;
 	struct net_device *ndev;
 	int ret = 0;
+	const struct emac_quirks *quirks;
 
 	ndev = alloc_etherdev(sizeof(struct emac_board_info));
 	if (!ndev) {
@@ -986,6 +998,13 @@ static int emac_probe(struct platform_device *pdev)
 
 	spin_lock_init(&db->lock);
 
+	quirks = of_device_get_match_data(&pdev->dev);
+	if (!quirks) {
+		dev_err(&pdev->dev, "Failed to determine the quirks to use\n");
+		ret = -ENODEV;
+		goto out;
+	}
+
 	db->membase = of_iomap(np, 0);
 	if (!db->membase) {
 		dev_err(&pdev->dev, "failed to remap registers\n");
@@ -1002,19 +1021,34 @@ static int emac_probe(struct platform_device *pdev)
 		goto out_iounmap;
 	}
 
+	if (quirks->has_reset) {
+		db->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+		if (IS_ERR(db->reset)) {
+			dev_err(&pdev->dev, "unable to request reset\n");
+			ret = PTR_ERR(db->reset);
+			goto out_dispose_mapping;
+		}
+
+		ret = reset_control_deassert(db->reset);
+		if (ret) {
+			dev_err(&pdev->dev, "could not deassert EMAC reset\n");
+			goto out_dispose_mapping;
+		}
+	}
+
 	if (emac_configure_dma(db))
 		netdev_info(ndev, "configure dma failed. disable dma.\n");
 
 	db->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(db->clk)) {
 		ret = PTR_ERR(db->clk);
-		goto out_dispose_mapping;
+		goto out_assert_reset;
 	}
 
 	ret = clk_prepare_enable(db->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "Error couldn't enable clock (%d)\n", ret);
-		goto out_dispose_mapping;
+		goto out_assert_reset;
 	}
 
 	ret = sunxi_sram_claim(&pdev->dev);
@@ -1070,6 +1104,8 @@ static int emac_probe(struct platform_device *pdev)
 	sunxi_sram_release(&pdev->dev);
 out_clk_disable_unprepare:
 	clk_disable_unprepare(db->clk);
+out_assert_reset:
+	reset_control_assert(db->reset);
 out_dispose_mapping:
 	irq_dispose_mapping(ndev->irq);
 out_iounmap:
@@ -1095,6 +1131,7 @@ static int emac_remove(struct platform_device *pdev)
 	unregister_netdev(ndev);
 	sunxi_sram_release(&pdev->dev);
 	clk_disable_unprepare(db->clk);
+	reset_control_assert(db->reset);
 	irq_dispose_mapping(ndev->irq);
 	iounmap(db->membase);
 	free_netdev(ndev);
@@ -1126,11 +1163,28 @@ static int emac_resume(struct platform_device *dev)
 	return 0;
 }
 
-static const struct of_device_id emac_of_match[] = {
-	{.compatible = "allwinner,sun4i-a10-emac",},
+static const struct emac_quirks sun4i_a10_emac_quirks = {
+	.has_reset = false,
+};
+
+static const struct emac_quirks sun8i_r40_emac_quirks = {
+	.has_reset = true,
+};
 
+static const struct of_device_id emac_of_match[] = {
+	{
+		.compatible = "allwinner,sun4i-a10-emac",
+		.data = &sun4i_a10_emac_quirks
+	},
+	{
+		.compatible = "allwinner,sun8i-r40-emac",
+		.data = &sun8i_r40_emac_quirks
+	},
 	/* Deprecated */
-	{.compatible = "allwinner,sun4i-emac",},
+	{
+		.compatible = "allwinner,sun4i-emac",
+		.data = &sun4i_a10_emac_quirks
+	},
 	{},
 };
 
-- 
2.25.1


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

[PATCH v5 2/3] dt-bindings: net: support for Allwinner R40 EMAC controller

[Evgeny Boger]

R40 and A10/A20 share the same EMAC IP.
However, on R40 the EMAC reset needs to be deasserted first,
so resets property is required.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 .../bindings/net/allwinner,sun4i-a10-emac.yaml  | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
index 8d8560a67abf..d9ed7b547af3 100644
--- a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
+++ b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
@@ -15,7 +15,9 @@ maintainers:
 
 properties:
   compatible:
-    const: allwinner,sun4i-a10-emac
+    enum:
+      - allwinner,sun4i-a10-emac
+      - allwinner,sun8i-r40-emac
 
   reg:
     maxItems: 1
@@ -30,6 +32,19 @@ properties:
     description: Phandle to the device SRAM
     $ref: /schemas/types.yaml#/definitions/phandle-array
 
+  resets:
+    maxItems: 1
+
+if:
+  properties:
+    compatible:
+      contains:
+        const: allwinner,sun8i-r40-emac
+
+then:
+  required:
+    - resets
+
 required:
   - compatible
   - reg
-- 
2.25.1

[PATCH v5 2/3] dt-bindings: net: support for Allwinner R40 EMAC controller

[Evgeny Boger]

R40 and A10/A20 share the same EMAC IP.
However, on R40 the EMAC reset needs to be deasserted first,
so resets property is required.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 .../bindings/net/allwinner,sun4i-a10-emac.yaml  | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
index 8d8560a67abf..d9ed7b547af3 100644
--- a/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
+++ b/Documentation/devicetree/bindings/net/allwinner,sun4i-a10-emac.yaml
@@ -15,7 +15,9 @@ maintainers:
 
 properties:
   compatible:
-    const: allwinner,sun4i-a10-emac
+    enum:
+      - allwinner,sun4i-a10-emac
+      - allwinner,sun8i-r40-emac
 
   reg:
     maxItems: 1
@@ -30,6 +32,19 @@ properties:
     description: Phandle to the device SRAM
     $ref: /schemas/types.yaml#/definitions/phandle-array
 
+  resets:
+    maxItems: 1
+
+if:
+  properties:
+    compatible:
+      contains:
+        const: allwinner,sun8i-r40-emac
+
+then:
+  required:
+    - resets
+
 required:
   - compatible
   - reg
-- 
2.25.1


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

[PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Evgeny Boger]

R40 (aka V40, A40i, T3) has two different Ethernet IPs
called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).

In contrast to A10/A20 where GMAC and EMAC share the same pins
making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
Both EMAC (on port H) and GMAC (on port A) can be then enabled at
the same time, allowing for two ethernet ports.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 03d3e5f45a09..8770b105f86e 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
 			#size-cells = <1>;
 			ranges;
 
+			sram_a: sram@0 {
+				compatible = "mmio-sram";
+				reg = <0x00000000 0xc000>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges = <0 0x00000000 0xc000>;
+
+				emac_sram: sram-section@8000 {
+					compatible = "allwinner,sun4i-a10-sram-a3-a4";
+					reg = <0x8000 0x4000>;
+				};
+			};
+
 			sram_c: sram@1d00000 {
 				compatible = "mmio-sram";
 				reg = <0x01d00000 0xd0000>;
@@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
 				drive-strength = <40>;
 			};
 
+			emac_pa_pins: emac-pa-pins {
+				pins = "PA0", "PA1", "PA2",
+				       "PA3", "PA4", "PA5", "PA6",
+				       "PA7", "PA8", "PA9", "PA10",
+				       "PA11", "PA12", "PA13", "PA14",
+				       "PA15", "PA16";
+				function = "emac";
+			};
+
+			emac_ph_pins: emac-ph-pins {
+				pins = "PH8", "PH9", "PH10", "PH11",
+				       "PH14", "PH15", "PH16", "PH17",
+				       "PH18","PH19", "PH20", "PH21",
+				       "PH22", "PH23", "PH24", "PH25",
+				       "PH26", "PH27";
+				function = "emac";
+			};
+
 			i2c0_pins: i2c0-pins {
 				pins = "PB0", "PB1";
 				function = "i2c0";
@@ -999,6 +1030,24 @@ gmac_mdio: mdio {
 			};
 		};
 
+		emac: ethernet@1c0b000 {
+			compatible = "allwinner,sun8i-r40-emac";
+			reg = <0x01c0b000 0x1000>;
+			interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_EMAC>;
+			resets = <&ccu RST_BUS_EMAC>;
+			allwinner,sram = <&emac_sram 1>;
+			status = "disabled";
+		};
+
+		emac_mdio: mdio@1c0b080 {
+			compatible = "allwinner,sun4i-a10-mdio";
+			reg = <0x01c0b080 0x14>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		mbus: dram-controller@1c62000 {
 			compatible = "allwinner,sun8i-r40-mbus";
 			reg = <0x01c62000 0x1000>;
-- 
2.25.1

[PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Evgeny Boger]

R40 (aka V40, A40i, T3) has two different Ethernet IPs
called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).

In contrast to A10/A20 where GMAC and EMAC share the same pins
making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
Both EMAC (on port H) and GMAC (on port A) can be then enabled at
the same time, allowing for two ethernet ports.

Signed-off-by: Evgeny Boger <boger@wirenboard.com>
---
 arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 03d3e5f45a09..8770b105f86e 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
 			#size-cells = <1>;
 			ranges;
 
+			sram_a: sram@0 {
+				compatible = "mmio-sram";
+				reg = <0x00000000 0xc000>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+				ranges = <0 0x00000000 0xc000>;
+
+				emac_sram: sram-section@8000 {
+					compatible = "allwinner,sun4i-a10-sram-a3-a4";
+					reg = <0x8000 0x4000>;
+				};
+			};
+
 			sram_c: sram@1d00000 {
 				compatible = "mmio-sram";
 				reg = <0x01d00000 0xd0000>;
@@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
 				drive-strength = <40>;
 			};
 
+			emac_pa_pins: emac-pa-pins {
+				pins = "PA0", "PA1", "PA2",
+				       "PA3", "PA4", "PA5", "PA6",
+				       "PA7", "PA8", "PA9", "PA10",
+				       "PA11", "PA12", "PA13", "PA14",
+				       "PA15", "PA16";
+				function = "emac";
+			};
+
+			emac_ph_pins: emac-ph-pins {
+				pins = "PH8", "PH9", "PH10", "PH11",
+				       "PH14", "PH15", "PH16", "PH17",
+				       "PH18","PH19", "PH20", "PH21",
+				       "PH22", "PH23", "PH24", "PH25",
+				       "PH26", "PH27";
+				function = "emac";
+			};
+
 			i2c0_pins: i2c0-pins {
 				pins = "PB0", "PB1";
 				function = "i2c0";
@@ -999,6 +1030,24 @@ gmac_mdio: mdio {
 			};
 		};
 
+		emac: ethernet@1c0b000 {
+			compatible = "allwinner,sun8i-r40-emac";
+			reg = <0x01c0b000 0x1000>;
+			interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_EMAC>;
+			resets = <&ccu RST_BUS_EMAC>;
+			allwinner,sram = <&emac_sram 1>;
+			status = "disabled";
+		};
+
+		emac_mdio: mdio@1c0b080 {
+			compatible = "allwinner,sun4i-a10-mdio";
+			reg = <0x01c0b080 0x14>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
+
 		mbus: dram-controller@1c62000 {
 			compatible = "allwinner,sun8i-r40-mbus";
 			reg = <0x01c62000 0x1000>;
-- 
2.25.1


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

Re: [PATCH v5 1/3] net: allwinner: reset control support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

On Thu, Jan 13, 2022 at 08:37:32AM +0300, Evgeny Boger wrote:
> R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC reset needs to be deasserted.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

Acked-by: Maxime Ripard <maxime@cerno.tech>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 1/3] net: allwinner: reset control support

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 297 bytes --]

On Thu, Jan 13, 2022 at 08:37:32AM +0300, Evgeny Boger wrote:
> R40 (aka V40/A40i/T3) and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC reset needs to be deasserted.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

Acked-by: Maxime Ripard <maxime@cerno.tech>

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH v5 2/3] dt-bindings: net: support for Allwinner R40 EMAC controller

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]

On Thu, Jan 13, 2022 at 08:37:33AM +0300, Evgeny Boger wrote:
> R40 and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC reset needs to be deasserted first,
> so resets property is required.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

Acked-by: Maxime Ripard <maxime@cerno.tech>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 2/3] dt-bindings: net: support for Allwinner R40 EMAC controller

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 320 bytes --]

On Thu, Jan 13, 2022 at 08:37:33AM +0300, Evgeny Boger wrote:
> R40 and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC reset needs to be deasserted first,
> so resets property is required.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>

Acked-by: Maxime Ripard <maxime@cerno.tech>

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2229 bytes --]

Hi,

On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
> R40 (aka V40, A40i, T3) has two different Ethernet IPs
> called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> 
> In contrast to A10/A20 where GMAC and EMAC share the same pins
> making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> the same time, allowing for two ethernet ports.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> index 03d3e5f45a09..8770b105f86e 100644
> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
>  			#size-cells = <1>;
>  			ranges;
>  
> +			sram_a: sram@0 {
> +				compatible = "mmio-sram";
> +				reg = <0x00000000 0xc000>;
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				ranges = <0 0x00000000 0xc000>;
> +
> +				emac_sram: sram-section@8000 {
> +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> +					reg = <0x8000 0x4000>;
> +				};
> +			};
> +
>  			sram_c: sram@1d00000 {
>  				compatible = "mmio-sram";
>  				reg = <0x01d00000 0xd0000>;
> @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
>  				drive-strength = <40>;
>  			};
>  
> +			emac_pa_pins: emac-pa-pins {
> +				pins = "PA0", "PA1", "PA2",
> +				       "PA3", "PA4", "PA5", "PA6",
> +				       "PA7", "PA8", "PA9", "PA10",
> +				       "PA11", "PA12", "PA13", "PA14",
> +				       "PA15", "PA16";
> +				function = "emac";
> +			};
> +
> +			emac_ph_pins: emac-ph-pins {
> +				pins = "PH8", "PH9", "PH10", "PH11",
> +				       "PH14", "PH15", "PH16", "PH17",
> +				       "PH18","PH19", "PH20", "PH21",
> +				       "PH22", "PH23", "PH24", "PH25",
> +				       "PH26", "PH27";
> +				function = "emac";
> +			};

There's 17 pins on the first group, but 18 on the second, is it intentional?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2229 bytes --]

Hi,

On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
> R40 (aka V40, A40i, T3) has two different Ethernet IPs
> called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> 
> In contrast to A10/A20 where GMAC and EMAC share the same pins
> making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> the same time, allowing for two ethernet ports.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> index 03d3e5f45a09..8770b105f86e 100644
> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
>  			#size-cells = <1>;
>  			ranges;
>  
> +			sram_a: sram@0 {
> +				compatible = "mmio-sram";
> +				reg = <0x00000000 0xc000>;
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				ranges = <0 0x00000000 0xc000>;
> +
> +				emac_sram: sram-section@8000 {
> +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> +					reg = <0x8000 0x4000>;
> +				};
> +			};
> +
>  			sram_c: sram@1d00000 {
>  				compatible = "mmio-sram";
>  				reg = <0x01d00000 0xd0000>;
> @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
>  				drive-strength = <40>;
>  			};
>  
> +			emac_pa_pins: emac-pa-pins {
> +				pins = "PA0", "PA1", "PA2",
> +				       "PA3", "PA4", "PA5", "PA6",
> +				       "PA7", "PA8", "PA9", "PA10",
> +				       "PA11", "PA12", "PA13", "PA14",
> +				       "PA15", "PA16";
> +				function = "emac";
> +			};
> +
> +			emac_ph_pins: emac-ph-pins {
> +				pins = "PH8", "PH9", "PH10", "PH11",
> +				       "PH14", "PH15", "PH16", "PH17",
> +				       "PH18","PH19", "PH20", "PH21",
> +				       "PH22", "PH23", "PH24", "PH25",
> +				       "PH26", "PH27";
> +				function = "emac";
> +			};

There's 17 pins on the first group, but 18 on the second, is it intentional?

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Andre Przywara]

On Fri, 14 Jan 2022 10:42:55 +0100
Maxime Ripard <maxime@cerno.tech> wrote:

Hi,

> On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
> > R40 (aka V40, A40i, T3) has two different Ethernet IPs
> > called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> > while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> > 
> > In contrast to A10/A20 where GMAC and EMAC share the same pins
> > making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> > Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> > the same time, allowing for two ethernet ports.
> > 
> > Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> > ---
> >  arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > index 03d3e5f45a09..8770b105f86e 100644
> > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
> >  			#size-cells = <1>;
> >  			ranges;
> >  
> > +			sram_a: sram@0 {
> > +				compatible = "mmio-sram";
> > +				reg = <0x00000000 0xc000>;
> > +				#address-cells = <1>;
> > +				#size-cells = <1>;
> > +				ranges = <0 0x00000000 0xc000>;
> > +
> > +				emac_sram: sram-section@8000 {
> > +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> > +					reg = <0x8000 0x4000>;
> > +				};
> > +			};
> > +
> >  			sram_c: sram@1d00000 {
> >  				compatible = "mmio-sram";
> >  				reg = <0x01d00000 0xd0000>;
> > @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
> >  				drive-strength = <40>;
> >  			};
> >  
> > +			emac_pa_pins: emac-pa-pins {
> > +				pins = "PA0", "PA1", "PA2",
> > +				       "PA3", "PA4", "PA5", "PA6",
> > +				       "PA7", "PA8", "PA9", "PA10",
> > +				       "PA11", "PA12", "PA13", "PA14",
> > +				       "PA15", "PA16";
> > +				function = "emac";
> > +			};
> > +
> > +			emac_ph_pins: emac-ph-pins {
> > +				pins = "PH8", "PH9", "PH10", "PH11",
> > +				       "PH14", "PH15", "PH16", "PH17",
> > +				       "PH18","PH19", "PH20", "PH21",
> > +				       "PH22", "PH23", "PH24", "PH25",
> > +				       "PH26", "PH27";
> > +				function = "emac";
> > +			};  
> 
> There's 17 pins on the first group, but 18 on the second, is it intentional?

Yeah, looks like PA17 is missing above. This pin is used for MII only, so
it is omitted from the existing gmac_rgmii_pins group.

Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
GMAC and EMAC, right? I wonder if we should add a group that connects all
pins needed for MII to the GMAC as well, so basically the above (with PA17
added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
that (also to those above?) to avoid blowing up the DTB needlessly.

Cheers,
Andre.

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Andre Przywara]

On Fri, 14 Jan 2022 10:42:55 +0100
Maxime Ripard <maxime@cerno.tech> wrote:

Hi,

> On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
> > R40 (aka V40, A40i, T3) has two different Ethernet IPs
> > called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> > while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> > 
> > In contrast to A10/A20 where GMAC and EMAC share the same pins
> > making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> > Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> > the same time, allowing for two ethernet ports.
> > 
> > Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> > ---
> >  arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > index 03d3e5f45a09..8770b105f86e 100644
> > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
> >  			#size-cells = <1>;
> >  			ranges;
> >  
> > +			sram_a: sram@0 {
> > +				compatible = "mmio-sram";
> > +				reg = <0x00000000 0xc000>;
> > +				#address-cells = <1>;
> > +				#size-cells = <1>;
> > +				ranges = <0 0x00000000 0xc000>;
> > +
> > +				emac_sram: sram-section@8000 {
> > +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> > +					reg = <0x8000 0x4000>;
> > +				};
> > +			};
> > +
> >  			sram_c: sram@1d00000 {
> >  				compatible = "mmio-sram";
> >  				reg = <0x01d00000 0xd0000>;
> > @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
> >  				drive-strength = <40>;
> >  			};
> >  
> > +			emac_pa_pins: emac-pa-pins {
> > +				pins = "PA0", "PA1", "PA2",
> > +				       "PA3", "PA4", "PA5", "PA6",
> > +				       "PA7", "PA8", "PA9", "PA10",
> > +				       "PA11", "PA12", "PA13", "PA14",
> > +				       "PA15", "PA16";
> > +				function = "emac";
> > +			};
> > +
> > +			emac_ph_pins: emac-ph-pins {
> > +				pins = "PH8", "PH9", "PH10", "PH11",
> > +				       "PH14", "PH15", "PH16", "PH17",
> > +				       "PH18","PH19", "PH20", "PH21",
> > +				       "PH22", "PH23", "PH24", "PH25",
> > +				       "PH26", "PH27";
> > +				function = "emac";
> > +			};  
> 
> There's 17 pins on the first group, but 18 on the second, is it intentional?

Yeah, looks like PA17 is missing above. This pin is used for MII only, so
it is omitted from the existing gmac_rgmii_pins group.

Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
GMAC and EMAC, right? I wonder if we should add a group that connects all
pins needed for MII to the GMAC as well, so basically the above (with PA17
added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
that (also to those above?) to avoid blowing up the DTB needlessly.

Cheers,
Andre.

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

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Evgeny Boger]

14.01.2022 13:37, Andre Przywara пишет:
> On Fri, 14 Jan 2022 10:42:55 +0100
> Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
>> On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
>>> R40 (aka V40, A40i, T3) has two different Ethernet IPs
>>> called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
>>> while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
>>>
>>> In contrast to A10/A20 where GMAC and EMAC share the same pins
>>> making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
>>> Both EMAC (on port H) and GMAC (on port A) can be then enabled at
>>> the same time, allowing for two ethernet ports.
>>>
>>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
>>> ---
>>>   arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
>>>   1 file changed, 49 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
>>> index 03d3e5f45a09..8770b105f86e 100644
>>> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
>>> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
>>> @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
>>>   			#size-cells = <1>;
>>>   			ranges;
>>>   
>>> +			sram_a: sram@0 {
>>> +				compatible = "mmio-sram";
>>> +				reg = <0x00000000 0xc000>;
>>> +				#address-cells = <1>;
>>> +				#size-cells = <1>;
>>> +				ranges = <0 0x00000000 0xc000>;
>>> +
>>> +				emac_sram: sram-section@8000 {
>>> +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
>>> +					reg = <0x8000 0x4000>;
>>> +				};
>>> +			};
>>> +
>>>   			sram_c: sram@1d00000 {
>>>   				compatible = "mmio-sram";
>>>   				reg = <0x01d00000 0xd0000>;
>>> @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
>>>   				drive-strength = <40>;
>>>   			};
>>>   
>>> +			emac_pa_pins: emac-pa-pins {
>>> +				pins = "PA0", "PA1", "PA2",
>>> +				       "PA3", "PA4", "PA5", "PA6",
>>> +				       "PA7", "PA8", "PA9", "PA10",
>>> +				       "PA11", "PA12", "PA13", "PA14",
>>> +				       "PA15", "PA16";
>>> +				function = "emac";
>>> +			};
>>> +
>>> +			emac_ph_pins: emac-ph-pins {
>>> +				pins = "PH8", "PH9", "PH10", "PH11",
>>> +				       "PH14", "PH15", "PH16", "PH17",
>>> +				       "PH18","PH19", "PH20", "PH21",
>>> +				       "PH22", "PH23", "PH24", "PH25",
>>> +				       "PH26", "PH27";
>>> +				function = "emac";
>>> +			};
>> There's 17 pins on the first group, but 18 on the second, is it intentional?
> Yeah, looks like PA17 is missing above. This pin is used for MII only, so
> it is omitted from the existing gmac_rgmii_pins group.
>
> Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
> GMAC and EMAC, right? I wonder if we should add a group that connects all
> pins needed for MII to the GMAC as well, so basically the above (with PA17
> added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
> that (also to those above?) to avoid blowing up the DTB needlessly.
Hi Andre,

No, it's not intentional, thank you for noticing that!
I haven't tried EMAC on port A with the latest patch, but it should be 
trivial to do.
The problem with TXERR signal is that it's kind of optional, so it's 
hard to notice if it doesn't work properly.

As for adding gmac_*mii*_pins node, I think it could be useful. Do you 
suggest to add it to the same series? Strictly speaking, it has nothing 
to do with R40 and second ethernet support. GMAC is ubiquitous among 
Allwinner SoCs, so I think it would make sense to add to all SoCs at once.

As for /omit-if-no-ref/ on pinctrl nodes, is there a policy on it? I 
mean there are people (ourselves included) who use device tree overlays 
a lot, both in bootloader and kernel, so it's not that harmless.


> Cheers,
> Andre.


-- 
С уважением,
     Евгений Богер / Evgeny Boger
     CTO, Wiren Board Team
     https://wirenboard.com/ru
     +7 495 150 66 19 (# 33)

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Evgeny Boger]

14.01.2022 13:37, Andre Przywara пишет:
> On Fri, 14 Jan 2022 10:42:55 +0100
> Maxime Ripard <maxime@cerno.tech> wrote:
>
> Hi,
>
>> On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
>>> R40 (aka V40, A40i, T3) has two different Ethernet IPs
>>> called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
>>> while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
>>>
>>> In contrast to A10/A20 where GMAC and EMAC share the same pins
>>> making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
>>> Both EMAC (on port H) and GMAC (on port A) can be then enabled at
>>> the same time, allowing for two ethernet ports.
>>>
>>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
>>> ---
>>>   arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
>>>   1 file changed, 49 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
>>> index 03d3e5f45a09..8770b105f86e 100644
>>> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
>>> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
>>> @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
>>>   			#size-cells = <1>;
>>>   			ranges;
>>>   
>>> +			sram_a: sram@0 {
>>> +				compatible = "mmio-sram";
>>> +				reg = <0x00000000 0xc000>;
>>> +				#address-cells = <1>;
>>> +				#size-cells = <1>;
>>> +				ranges = <0 0x00000000 0xc000>;
>>> +
>>> +				emac_sram: sram-section@8000 {
>>> +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
>>> +					reg = <0x8000 0x4000>;
>>> +				};
>>> +			};
>>> +
>>>   			sram_c: sram@1d00000 {
>>>   				compatible = "mmio-sram";
>>>   				reg = <0x01d00000 0xd0000>;
>>> @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
>>>   				drive-strength = <40>;
>>>   			};
>>>   
>>> +			emac_pa_pins: emac-pa-pins {
>>> +				pins = "PA0", "PA1", "PA2",
>>> +				       "PA3", "PA4", "PA5", "PA6",
>>> +				       "PA7", "PA8", "PA9", "PA10",
>>> +				       "PA11", "PA12", "PA13", "PA14",
>>> +				       "PA15", "PA16";
>>> +				function = "emac";
>>> +			};
>>> +
>>> +			emac_ph_pins: emac-ph-pins {
>>> +				pins = "PH8", "PH9", "PH10", "PH11",
>>> +				       "PH14", "PH15", "PH16", "PH17",
>>> +				       "PH18","PH19", "PH20", "PH21",
>>> +				       "PH22", "PH23", "PH24", "PH25",
>>> +				       "PH26", "PH27";
>>> +				function = "emac";
>>> +			};
>> There's 17 pins on the first group, but 18 on the second, is it intentional?
> Yeah, looks like PA17 is missing above. This pin is used for MII only, so
> it is omitted from the existing gmac_rgmii_pins group.
>
> Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
> GMAC and EMAC, right? I wonder if we should add a group that connects all
> pins needed for MII to the GMAC as well, so basically the above (with PA17
> added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
> that (also to those above?) to avoid blowing up the DTB needlessly.
Hi Andre,

No, it's not intentional, thank you for noticing that!
I haven't tried EMAC on port A with the latest patch, but it should be 
trivial to do.
The problem with TXERR signal is that it's kind of optional, so it's 
hard to notice if it doesn't work properly.

As for adding gmac_*mii*_pins node, I think it could be useful. Do you 
suggest to add it to the same series? Strictly speaking, it has nothing 
to do with R40 and second ethernet support. GMAC is ubiquitous among 
Allwinner SoCs, so I think it would make sense to add to all SoCs at once.

As for /omit-if-no-ref/ on pinctrl nodes, is there a policy on it? I 
mean there are people (ourselves included) who use device tree overlays 
a lot, both in bootloader and kernel, so it's not that harmless.


> Cheers,
> Andre.


-- 
С уважением,
     Евгений Богер / Evgeny Boger
     CTO, Wiren Board Team
     https://wirenboard.com/ru
     +7 495 150 66 19 (# 33)


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

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4402 bytes --]

Hi,

On Fri, Jan 14, 2022 at 03:10:34PM +0300, Evgeny Boger wrote:
> 14.01.2022 13:37, Andre Przywara пишет:
> > On Fri, 14 Jan 2022 10:42:55 +0100
> > Maxime Ripard <maxime@cerno.tech> wrote:
> > 
> > Hi,
> > 
> > > On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
> > > > R40 (aka V40, A40i, T3) has two different Ethernet IPs
> > > > called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> > > > while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> > > > 
> > > > In contrast to A10/A20 where GMAC and EMAC share the same pins
> > > > making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> > > > Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> > > > the same time, allowing for two ethernet ports.
> > > > 
> > > > Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> > > > ---
> > > >   arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
> > > >   1 file changed, 49 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > index 03d3e5f45a09..8770b105f86e 100644
> > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
> > > >   			#size-cells = <1>;
> > > >   			ranges;
> > > > +			sram_a: sram@0 {
> > > > +				compatible = "mmio-sram";
> > > > +				reg = <0x00000000 0xc000>;
> > > > +				#address-cells = <1>;
> > > > +				#size-cells = <1>;
> > > > +				ranges = <0 0x00000000 0xc000>;
> > > > +
> > > > +				emac_sram: sram-section@8000 {
> > > > +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> > > > +					reg = <0x8000 0x4000>;
> > > > +				};
> > > > +			};
> > > > +
> > > >   			sram_c: sram@1d00000 {
> > > >   				compatible = "mmio-sram";
> > > >   				reg = <0x01d00000 0xd0000>;
> > > > @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
> > > >   				drive-strength = <40>;
> > > >   			};
> > > > +			emac_pa_pins: emac-pa-pins {
> > > > +				pins = "PA0", "PA1", "PA2",
> > > > +				       "PA3", "PA4", "PA5", "PA6",
> > > > +				       "PA7", "PA8", "PA9", "PA10",
> > > > +				       "PA11", "PA12", "PA13", "PA14",
> > > > +				       "PA15", "PA16";
> > > > +				function = "emac";
> > > > +			};
> > > > +
> > > > +			emac_ph_pins: emac-ph-pins {
> > > > +				pins = "PH8", "PH9", "PH10", "PH11",
> > > > +				       "PH14", "PH15", "PH16", "PH17",
> > > > +				       "PH18","PH19", "PH20", "PH21",
> > > > +				       "PH22", "PH23", "PH24", "PH25",
> > > > +				       "PH26", "PH27";
> > > > +				function = "emac";
> > > > +			};
> > > There's 17 pins on the first group, but 18 on the second, is it intentional?
> > Yeah, looks like PA17 is missing above. This pin is used for MII only, so
> > it is omitted from the existing gmac_rgmii_pins group.
> > 
> > Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
> > GMAC and EMAC, right? I wonder if we should add a group that connects all
> > pins needed for MII to the GMAC as well, so basically the above (with PA17
> > added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
> > that (also to those above?) to avoid blowing up the DTB needlessly.
>
> No, it's not intentional, thank you for noticing that!
> I haven't tried EMAC on port A with the latest patch, but it should be
> trivial to do.
> The problem with TXERR signal is that it's kind of optional, so it's hard to
> notice if it doesn't work properly.

TXERR seems to be rarely used for our boards (there's only one in tree,
plus the A20-marsboard currently discussed) so having it as a separate
pinctrl node would make more sense.

> As for adding gmac_*mii*_pins node, I think it could be useful. Do you
> suggest to add it to the same series? Strictly speaking, it has nothing to
> do with R40 and second ethernet support. GMAC is ubiquitous among Allwinner
> SoCs, so I think it would make sense to add to all SoCs at once.
> 
> As for /omit-if-no-ref/ on pinctrl nodes, is there a policy on it? I mean
> there are people (ourselves included) who use device tree overlays a lot,
> both in bootloader and kernel, so it's not that harmless.

/omit-if-no-ref/ doesn't do anything if the DT is compiled with overlay
support (dtc -@)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 4402 bytes --]

Hi,

On Fri, Jan 14, 2022 at 03:10:34PM +0300, Evgeny Boger wrote:
> 14.01.2022 13:37, Andre Przywara пишет:
> > On Fri, 14 Jan 2022 10:42:55 +0100
> > Maxime Ripard <maxime@cerno.tech> wrote:
> > 
> > Hi,
> > 
> > > On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:
> > > > R40 (aka V40, A40i, T3) has two different Ethernet IPs
> > > > called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> > > > while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> > > > 
> > > > In contrast to A10/A20 where GMAC and EMAC share the same pins
> > > > making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> > > > Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> > > > the same time, allowing for two ethernet ports.
> > > > 
> > > > Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> > > > ---
> > > >   arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
> > > >   1 file changed, 49 insertions(+)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > index 03d3e5f45a09..8770b105f86e 100644
> > > > --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> > > > @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
> > > >   			#size-cells = <1>;
> > > >   			ranges;
> > > > +			sram_a: sram@0 {
> > > > +				compatible = "mmio-sram";
> > > > +				reg = <0x00000000 0xc000>;
> > > > +				#address-cells = <1>;
> > > > +				#size-cells = <1>;
> > > > +				ranges = <0 0x00000000 0xc000>;
> > > > +
> > > > +				emac_sram: sram-section@8000 {
> > > > +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> > > > +					reg = <0x8000 0x4000>;
> > > > +				};
> > > > +			};
> > > > +
> > > >   			sram_c: sram@1d00000 {
> > > >   				compatible = "mmio-sram";
> > > >   				reg = <0x01d00000 0xd0000>;
> > > > @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
> > > >   				drive-strength = <40>;
> > > >   			};
> > > > +			emac_pa_pins: emac-pa-pins {
> > > > +				pins = "PA0", "PA1", "PA2",
> > > > +				       "PA3", "PA4", "PA5", "PA6",
> > > > +				       "PA7", "PA8", "PA9", "PA10",
> > > > +				       "PA11", "PA12", "PA13", "PA14",
> > > > +				       "PA15", "PA16";
> > > > +				function = "emac";
> > > > +			};
> > > > +
> > > > +			emac_ph_pins: emac-ph-pins {
> > > > +				pins = "PH8", "PH9", "PH10", "PH11",
> > > > +				       "PH14", "PH15", "PH16", "PH17",
> > > > +				       "PH18","PH19", "PH20", "PH21",
> > > > +				       "PH22", "PH23", "PH24", "PH25",
> > > > +				       "PH26", "PH27";
> > > > +				function = "emac";
> > > > +			};
> > > There's 17 pins on the first group, but 18 on the second, is it intentional?
> > Yeah, looks like PA17 is missing above. This pin is used for MII only, so
> > it is omitted from the existing gmac_rgmii_pins group.
> > 
> > Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
> > GMAC and EMAC, right? I wonder if we should add a group that connects all
> > pins needed for MII to the GMAC as well, so basically the above (with PA17
> > added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
> > that (also to those above?) to avoid blowing up the DTB needlessly.
>
> No, it's not intentional, thank you for noticing that!
> I haven't tried EMAC on port A with the latest patch, but it should be
> trivial to do.
> The problem with TXERR signal is that it's kind of optional, so it's hard to
> notice if it doesn't work properly.

TXERR seems to be rarely used for our boards (there's only one in tree,
plus the A20-marsboard currently discussed) so having it as a separate
pinctrl node would make more sense.

> As for adding gmac_*mii*_pins node, I think it could be useful. Do you
> suggest to add it to the same series? Strictly speaking, it has nothing to
> do with R40 and second ethernet support. GMAC is ubiquitous among Allwinner
> SoCs, so I think it would make sense to add to all SoCs at once.
> 
> As for /omit-if-no-ref/ on pinctrl nodes, is there a policy on it? I mean
> there are people (ourselves included) who use device tree overlays a lot,
> both in bootloader and kernel, so it's not that harmless.

/omit-if-no-ref/ doesn't do anything if the DT is compiled with overlay
support (dtc -@)

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Andre Przywara]

On Fri, 14 Jan 2022 15:10:34 +0300
Evgeny Boger <boger@wirenboard.com> wrote:

Hi,

> 14.01.2022 13:37, Andre Przywara пишет:
> > On Fri, 14 Jan 2022 10:42:55 +0100
> > Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >  
> >> On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:  
> >>> R40 (aka V40, A40i, T3) has two different Ethernet IPs
> >>> called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> >>> while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> >>>
> >>> In contrast to A10/A20 where GMAC and EMAC share the same pins
> >>> making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> >>> Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> >>> the same time, allowing for two ethernet ports.
> >>>
> >>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> >>> ---
> >>>   arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
> >>>   1 file changed, 49 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> >>> index 03d3e5f45a09..8770b105f86e 100644
> >>> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> >>> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> >>> @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
> >>>   			#size-cells = <1>;
> >>>   			ranges;
> >>>   
> >>> +			sram_a: sram@0 {
> >>> +				compatible = "mmio-sram";
> >>> +				reg = <0x00000000 0xc000>;
> >>> +				#address-cells = <1>;
> >>> +				#size-cells = <1>;
> >>> +				ranges = <0 0x00000000 0xc000>;
> >>> +
> >>> +				emac_sram: sram-section@8000 {
> >>> +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> >>> +					reg = <0x8000 0x4000>;
> >>> +				};
> >>> +			};
> >>> +
> >>>   			sram_c: sram@1d00000 {
> >>>   				compatible = "mmio-sram";
> >>>   				reg = <0x01d00000 0xd0000>;
> >>> @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
> >>>   				drive-strength = <40>;
> >>>   			};
> >>>   
> >>> +			emac_pa_pins: emac-pa-pins {
> >>> +				pins = "PA0", "PA1", "PA2",
> >>> +				       "PA3", "PA4", "PA5", "PA6",
> >>> +				       "PA7", "PA8", "PA9", "PA10",
> >>> +				       "PA11", "PA12", "PA13", "PA14",
> >>> +				       "PA15", "PA16";
> >>> +				function = "emac";
> >>> +			};
> >>> +
> >>> +			emac_ph_pins: emac-ph-pins {
> >>> +				pins = "PH8", "PH9", "PH10", "PH11",
> >>> +				       "PH14", "PH15", "PH16", "PH17",
> >>> +				       "PH18","PH19", "PH20", "PH21",
> >>> +				       "PH22", "PH23", "PH24", "PH25",
> >>> +				       "PH26", "PH27";
> >>> +				function = "emac";
> >>> +			};  
> >> There's 17 pins on the first group, but 18 on the second, is it intentional?  
> > Yeah, looks like PA17 is missing above. This pin is used for MII only, so
> > it is omitted from the existing gmac_rgmii_pins group.
> >
> > Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
> > GMAC and EMAC, right? I wonder if we should add a group that connects all
> > pins needed for MII to the GMAC as well, so basically the above (with PA17
> > added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
> > that (also to those above?) to avoid blowing up the DTB needlessly.  
> Hi Andre,
> 
> No, it's not intentional, thank you for noticing that!

Kudos go to Maxime for spotting the different number of pins!

> I haven't tried EMAC on port A with the latest patch, but it should be 
> trivial to do.
> The problem with TXERR signal is that it's kind of optional, so it's 
> hard to notice if it doesn't work properly.

I see, I was wondering already, since you mentioned MII on both ports
before, IIRC.

> As for adding gmac_*mii*_pins node, I think it could be useful. Do you 
> suggest to add it to the same series? Strictly speaking, it has nothing 
> to do with R40 and second ethernet support. GMAC is ubiquitous among 
> Allwinner SoCs, so I think it would make sense to add to all SoCs at once.

In general we try to add those pin groups only when they get used. The A64
describes both sets of pins (MII & RGMII), because there is the Pine64
(non-plus) with a Realtek 8201FN PHY. For the H3 & H6 this was not needed
before, since board vendors use the internal PHY for 100MBit. I think
similar reasoning applies to the other SoCs.

> As for /omit-if-no-ref/ on pinctrl nodes, is there a policy on it?

Not written out, but my understanding is that if there is only a very
small number of users, we try not to bother everyone else. And connecting
a PHY via MII to the GMAC sounds like a niche use case, if you have the
EMAC on PortH (now).

> I 
> mean there are people (ourselves included) who use device tree overlays 
> a lot, both in bootloader and kernel, so it's not that harmless.

I haven't checked, do those nodes still get removed if you compile the DT
with -@?

Cheers,
Andre

Re: [PATCH v5 3/3] ARM: dts: sun8i: r40: add second ethernet support

[Andre Przywara]

On Fri, 14 Jan 2022 15:10:34 +0300
Evgeny Boger <boger@wirenboard.com> wrote:

Hi,

> 14.01.2022 13:37, Andre Przywara пишет:
> > On Fri, 14 Jan 2022 10:42:55 +0100
> > Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >  
> >> On Thu, Jan 13, 2022 at 08:37:34AM +0300, Evgeny Boger wrote:  
> >>> R40 (aka V40, A40i, T3) has two different Ethernet IPs
> >>> called EMAC and GMAC. EMAC only support 10/100 Mbit in MII mode,
> >>> while GMAC support both 10/100 (MII) and 10/100/1000 (RGMII).
> >>>
> >>> In contrast to A10/A20 where GMAC and EMAC share the same pins
> >>> making EMAC somewhat pointless, on R40 EMAC can be routed to port H.
> >>> Both EMAC (on port H) and GMAC (on port A) can be then enabled at
> >>> the same time, allowing for two ethernet ports.
> >>>
> >>> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> >>> ---
> >>>   arch/arm/boot/dts/sun8i-r40.dtsi | 49 ++++++++++++++++++++++++++++++++
> >>>   1 file changed, 49 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> >>> index 03d3e5f45a09..8770b105f86e 100644
> >>> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> >>> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> >>> @@ -217,6 +217,19 @@ syscon: system-control@1c00000 {
> >>>   			#size-cells = <1>;
> >>>   			ranges;
> >>>   
> >>> +			sram_a: sram@0 {
> >>> +				compatible = "mmio-sram";
> >>> +				reg = <0x00000000 0xc000>;
> >>> +				#address-cells = <1>;
> >>> +				#size-cells = <1>;
> >>> +				ranges = <0 0x00000000 0xc000>;
> >>> +
> >>> +				emac_sram: sram-section@8000 {
> >>> +					compatible = "allwinner,sun4i-a10-sram-a3-a4";
> >>> +					reg = <0x8000 0x4000>;
> >>> +				};
> >>> +			};
> >>> +
> >>>   			sram_c: sram@1d00000 {
> >>>   				compatible = "mmio-sram";
> >>>   				reg = <0x01d00000 0xd0000>;
> >>> @@ -553,6 +566,24 @@ gmac_rgmii_pins: gmac-rgmii-pins {
> >>>   				drive-strength = <40>;
> >>>   			};
> >>>   
> >>> +			emac_pa_pins: emac-pa-pins {
> >>> +				pins = "PA0", "PA1", "PA2",
> >>> +				       "PA3", "PA4", "PA5", "PA6",
> >>> +				       "PA7", "PA8", "PA9", "PA10",
> >>> +				       "PA11", "PA12", "PA13", "PA14",
> >>> +				       "PA15", "PA16";
> >>> +				function = "emac";
> >>> +			};
> >>> +
> >>> +			emac_ph_pins: emac-ph-pins {
> >>> +				pins = "PH8", "PH9", "PH10", "PH11",
> >>> +				       "PH14", "PH15", "PH16", "PH17",
> >>> +				       "PH18","PH19", "PH20", "PH21",
> >>> +				       "PH22", "PH23", "PH24", "PH25",
> >>> +				       "PH26", "PH27";
> >>> +				function = "emac";
> >>> +			};  
> >> There's 17 pins on the first group, but 18 on the second, is it intentional?  
> > Yeah, looks like PA17 is missing above. This pin is used for MII only, so
> > it is omitted from the existing gmac_rgmii_pins group.
> >
> > Evgeny: Did you try a 100MBit PHY on PortA? That should work with both the
> > GMAC and EMAC, right? I wonder if we should add a group that connects all
> > pins needed for MII to the GMAC as well, so basically the above (with PA17
> > added), but using 'function = "gmac";'? Put an "/omit-if-no-ref/" before
> > that (also to those above?) to avoid blowing up the DTB needlessly.  
> Hi Andre,
> 
> No, it's not intentional, thank you for noticing that!

Kudos go to Maxime for spotting the different number of pins!

> I haven't tried EMAC on port A with the latest patch, but it should be 
> trivial to do.
> The problem with TXERR signal is that it's kind of optional, so it's 
> hard to notice if it doesn't work properly.

I see, I was wondering already, since you mentioned MII on both ports
before, IIRC.

> As for adding gmac_*mii*_pins node, I think it could be useful. Do you 
> suggest to add it to the same series? Strictly speaking, it has nothing 
> to do with R40 and second ethernet support. GMAC is ubiquitous among 
> Allwinner SoCs, so I think it would make sense to add to all SoCs at once.

In general we try to add those pin groups only when they get used. The A64
describes both sets of pins (MII & RGMII), because there is the Pine64
(non-plus) with a Realtek 8201FN PHY. For the H3 & H6 this was not needed
before, since board vendors use the internal PHY for 100MBit. I think
similar reasoning applies to the other SoCs.

> As for /omit-if-no-ref/ on pinctrl nodes, is there a policy on it?

Not written out, but my understanding is that if there is only a very
small number of users, we try not to bother everyone else. And connecting
a PHY via MII to the GMAC sounds like a niche use case, if you have the
EMAC on PortH (now).

> I 
> mean there are people (ourselves included) who use device tree overlays 
> a lot, both in bootloader and kernel, so it's not that harmless.

I haven't checked, do those nodes still get removed if you compile the DT
with -@?

Cheers,
Andre

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

Re: [PATCH v5 2/3] dt-bindings: net: support for Allwinner R40 EMAC controller

[Rob Herring]

On Thu, 13 Jan 2022 08:37:33 +0300, Evgeny Boger wrote:
> R40 and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC reset needs to be deasserted first,
> so resets property is required.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> ---
>  .../bindings/net/allwinner,sun4i-a10-emac.yaml  | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v5 2/3] dt-bindings: net: support for Allwinner R40 EMAC controller

[Rob Herring]

On Thu, 13 Jan 2022 08:37:33 +0300, Evgeny Boger wrote:
> R40 and A10/A20 share the same EMAC IP.
> However, on R40 the EMAC reset needs to be deasserted first,
> so resets property is required.
> 
> Signed-off-by: Evgeny Boger <boger@wirenboard.com>
> ---
>  .../bindings/net/allwinner,sun4i-a10-emac.yaml  | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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