Skip to contentSkip to conversation

Introduce EN7581 ethernet support

PATCH Superseded Lorenzo Bianconi 6 participants 19 messages Jun 23, 2024 – Jun 28, 2024

Subsystems (5)
Networking Network drivers Device tree bindings MediaTek Ethernet driver +1 more subsystems

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Lorenzo Bianconi

author maintainer
#0 lore raw

v3 cover Introduce EN7581 ethernet support

Add airoha_eth driver in order to introduce ethernet support for Airoha EN7581 SoC available on EN7581 development board. EN7581 mac controller is mainly composed by Frame Engine (FE) and QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic functionalities are supported now) while QDMA is used for DMA operation and QOS functionalities between mac layer and the dsa switch (hw QoS is not available yet and it will be added in the future). Currently only hw lan features are available, hw wan will be added with subsequent patches.

Changes since v2:
- rename airoha,en7581.yaml in airoha,en7581-eth.yaml
- remove reset dependency in airoha,en7581-eth.yaml
- remove airoha_dev_change_mtu() callback
Changes since v1:
- drop patch 2/3
- remove queue lock for rx queues
- add bql support
- add ethtool stats support
- fix possible infinite loop in airoha_qdma_rx_process routine
- always destroy page_pool in case of error during initialization
- cosmetics

Lorenzo Bianconi (2):
dt-bindings: net: airoha: Add EN7581 ethernet controller
net: airoha: Introduce ethernet support for EN7581 SoC

 .../bindings/net/airoha,en7581-eth.yaml       |  108 +
 MAINTAINERS                                   |   10 +
 drivers/net/ethernet/mediatek/Kconfig         |   11 +-
 drivers/net/ethernet/mediatek/Makefile        |    1 +
 drivers/net/ethernet/mediatek/airoha_eth.c    | 1783 +++++++++++++++++
 drivers/net/ethernet/mediatek/airoha_eth.h    |  793 ++++++++
 6 files changed, 2705 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
 create mode 100644 drivers/net/ethernet/mediatek/airoha_eth.c
 create mode 100644 drivers/net/ethernet/mediatek/airoha_eth.h
Signature · 1 line
2.45.2
author maintainer
#1 lore raw

PATCH v3 1/2 dt-bindings: net: airoha: Add EN7581 ethernet controller

Introduce device-tree binding documentation for Airoha EN7581 ethernet mac controller.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../bindings/net/airoha,en7581-eth.yaml       | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +108 -0
new file mode 100644
index 000000000000..e25a462a75d4
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7581 Frame Engine Ethernet controller
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+maintainers:
+  - Lorenzo Bianconi <[email protected]>
+
+description:
+  The frame engine ethernet controller can be found on Airoha SoCs.
+  These SoCs have dual GMAC ports.
+
+properties:
+  compatible:
+    enum:
+      - airoha,en7581-eth
+
+  reg:
+    items:
+      - description: Frame engine base address
+      - description: QDMA0 base address
+      - description: QDMA1 base address
+
+  reg-names:
+    items:
+      - const: fe
+      - const: qdma0
+      - const: qdma1
+
+  interrupts:
+    maxItems: 10
+
+  resets:
+    maxItems: 8
+
+  reset-names:
+    items:
+      - const: fe
+      - const: pdma
+      - const: qdma
+      - const: xsi-mac
+      - const: hsi0-mac
+      - const: hsi1-mac
+      - const: hsi-mac
+      - const: xfp-mac
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/en7523-clk.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      eth0: ethernet@1fb50000 {
+        compatible = "airoha,en7581-eth";
+        reg = <0 0x1fb50000 0 0x2600>,
+              <0 0x1fb54000 0 0x2000>,
+              <0 0x1fb56000 0 0x2000>;
+        reg-names = "fe", "qdma0", "qdma1";
+
+        resets = <&scuclk 44>,
+                 <&scuclk 30>,
+                 <&scuclk 31>,
+                 <&scuclk 6>,
+                 <&scuclk 15>,
+                 <&scuclk 16>,
+                 <&scuclk 17>,
+                 <&scuclk 26>;
+        reset-names = "fe", "pdma", "qdma", "xsi-mac",
+                      "hsi0-mac", "hsi1-mac", "hsi-mac",
+                      "xfp-mac";
+
+        interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+      };
+    };
Signature · 1 line
2.45.2
maintainer
#2 lore raw

Re: [PATCH v3 net-next 1/2] dt-bindings: net: airoha: Add EN7581 ethernet controller

Quoted 42 lines · Lorenzo Bianconi

Introduce device-tree binding documentation for Airoha EN7581 ethernet mac controller.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../bindings/net/airoha,en7581-eth.yaml       | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +27 -0
new file mode 100644
index 000000000000..e25a462a75d4
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7581 Frame Engine Ethernet controller
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+maintainers:
+  - Lorenzo Bianconi <[email protected]>
+
+description:
+  The frame engine ethernet controller can be found on Airoha SoCs.
+  These SoCs have dual GMAC ports.
+
+properties:
+  compatible:
+    enum:
+      - airoha,en7581-eth
+
+  reg:
+    items:
+      - description: Frame engine base address
+      - description: QDMA0 base address

Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml line 37

Quoted 10 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +10 -0
+      - description: QDMA1 base address
+
+  reg-names:
+    items:
+      - const: fe
+      - const: qdma0
+      - const: qdma1
+
+  interrupts:
+    maxItems: 10

You need to define what each interrupt is. Just like 'reg'.

Quoted 11 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +11 -0
+
+  resets:
+    maxItems: 8
+
+  reset-names:
+    items:
+      - const: fe
+      - const: pdma
+      - const: qdma
+      - const: xsi-mac
+      - const: hsi0-mac

Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml line 57

Quoted 10 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +9 -0
+      - const: hsi1-mac
+      - const: hsi-mac
+      - const: xfp-mac
+
 
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0

What are these for? You have no child nodes to use them.

Quoted 54 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +51 -0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/en7523-clk.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      eth0: ethernet@1fb50000 {
+        compatible = "airoha,en7581-eth";
+        reg = <0 0x1fb50000 0 0x2600>,
+              <0 0x1fb54000 0 0x2000>,
+              <0 0x1fb56000 0 0x2000>;
+        reg-names = "fe", "qdma0", "qdma1";
+
+        resets = <&scuclk 44>,
+                 <&scuclk 30>,
+                 <&scuclk 31>,
+                 <&scuclk 6>,
+                 <&scuclk 15>,
+                 <&scuclk 16>,
+                 <&scuclk 17>,
+                 <&scuclk 26>;
+        reset-names = "fe", "pdma", "qdma", "xsi-mac",
+                      "hsi0-mac", "hsi1-mac", "hsi-mac",
+                      "xfp-mac";
+
+        interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+      };
+    };
Signature · 1 line
2.45.2
author maintainer
#3 lore raw

Re: [PATCH v3 net-next 1/2] dt-bindings: net: airoha: Add EN7581 ethernet controller

Quoted 47 lines · Lorenzo Bianconi
Quoted 46 lines

Introduce device-tree binding documentation for Airoha EN7581 ethernet mac controller.

Signed-off-by Lorenzo Bianconi <[email protected]>
---
 .../bindings/net/airoha,en7581-eth.yaml       | 108 ++++++++++++++++++
 1 file changed, 108 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +31 -0
new file mode 100644
index 000000000000..e25a462a75d4
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Airoha EN7581 Frame Engine Ethernet controller
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+maintainers:
+  - Lorenzo Bianconi <[email protected]>
+
+description:
+  The frame engine ethernet controller can be found on Airoha SoCs.
+  These SoCs have dual GMAC ports.
+
+properties:
+  compatible:
+    enum:
+      - airoha,en7581-eth
+
+  reg:
+    items:
+      - description: Frame engine base address
+      - description: QDMA0 base address
+      - description: QDMA1 base address
+
+  reg-names:
+    items:
Quoted 8 lines

Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml line 37

Quoted 6 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +6 -0
+      - const: fe
+      - const: qdma0
+      - const: qdma1
+
+  interrupts:
+    maxItems: 10

You need to define what each interrupt is. Just like 'reg'.

ack, I will fix it in v4.

Quoted 17 lines
Quoted 16 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +15 -0
+
+  resets:
+    maxItems: 8
+
+  reset-names:
+    items:
+      - const: fe
+      - const: pdma
+      - const: qdma
+      - const: xsi-mac
+      - const: hsi0-mac
+      - const: hsi1-mac
+      - const: hsi-mac
+      - const: xfp-mac
+
Quoted 7 lines

Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml line 57

Quoted 5 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +5 -0
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0

What are these for? You have no child nodes to use them.

ack, in v4 I will add mac ethernet subnode.

Regards,
Lorenzo

Quoted 55 lines
Quoted 55 lines
Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml new file +51 -0
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/en7523-clk.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      eth0: ethernet@1fb50000 {
+        compatible = "airoha,en7581-eth";
+        reg = <0 0x1fb50000 0 0x2600>,
+              <0 0x1fb54000 0 0x2000>,
+              <0 0x1fb56000 0 0x2000>;
+        reg-names = "fe", "qdma0", "qdma1";
+
+        resets = <&scuclk 44>,
+                 <&scuclk 30>,
+                 <&scuclk 31>,
+                 <&scuclk 6>,
+                 <&scuclk 15>,
+                 <&scuclk 16>,
+                 <&scuclk 17>,
+                 <&scuclk 26>;
+        reset-names = "fe", "pdma", "qdma", "xsi-mac",
+                      "hsi0-mac", "hsi1-mac", "hsi-mac",
+                      "xfp-mac";
+
+        interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
+      };
+    };
Signature · 1 line
2.45.2
author maintainer
#4 lore raw

PATCH v3 2/2 net: airoha: Introduce ethernet support for EN7581 SoC

In reply to Lorenzo Bianconi

Add airoha_eth driver in order to introduce ethernet support for Airoha EN7581 SoC available on EN7581 development board (en7581-evb). en7581-evb networking architecture is composed by airoha_eth as mac controller (cpu port) and a mt7530 dsa based switch.
EN7581 mac controller is mainly composed by Frame Engine (FE) and QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic functionalities are supported now) while QDMA is used for DMA operation and QOS functionalities between mac layer and the dsa switch (hw QoS is not available yet and it will be added in the future). Currently only hw lan features are available, hw wan will be added with subsequent patches.

Tested-by Benjamin Larsson <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>
---
 MAINTAINERS                                |   10 +
 drivers/net/ethernet/mediatek/Kconfig      |   11 +-
 drivers/net/ethernet/mediatek/Makefile     |    1 +
 drivers/net/ethernet/mediatek/airoha_eth.c | 1783 ++++++++++++++++++++
 drivers/net/ethernet/mediatek/airoha_eth.h |  793 +++++++++
 5 files changed, 2597 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/mediatek/airoha_eth.c
 create mode 100644 drivers/net/ethernet/mediatek/airoha_eth.h
MAINTAINERS +10 -0
index 0e3cb040fc16..c44169beb8f1 100644
@@ -682,6 +682,16 @@ S:	Supported
 F:	fs/aio.c
 F:	include/linux/*aio*.h
 
+AIROHA ETHERNET DRIVER
+M:	Lorenzo Bianconi <[email protected]>
+L:	[email protected] (moderated for non-subscribers)
+L:	[email protected] (moderated for non-subscribers)
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
+F:	drivers/net/ethernet/mediatek/airoha_eth.c
+F:	drivers/net/ethernet/mediatek/airoha_eth.h
+
 AIROHA SPI SNFI DRIVER
 M:	Lorenzo Bianconi <[email protected]>
 M:	Ray Liu <[email protected]>
drivers/net/ethernet/mediatek/Kconfig +10 -1
index da0db417ab69..e16d0ea5d418 100644
@@ -1,12 +1,21 @@
 # SPDX-License-Identifier: GPL-2.0-only
 config NET_VENDOR_MEDIATEK
 	bool "MediaTek devices"
-	depends on ARCH_MEDIATEK || SOC_MT7621 || SOC_MT7620 || COMPILE_TEST
+	depends on ARCH_MEDIATEK || ARCH_AIROHA || SOC_MT7621 || SOC_MT7620 || COMPILE_TEST
 	help
 	  If you have a Mediatek SoC with ethernet, say Y.
 
 if NET_VENDOR_MEDIATEK
 
+config NET_AIROHA
+	tristate "Airoha SoC Gigabit Ethernet support"
+	depends on NET_DSA || !NET_DSA
+	select PAGE_POOL
+	select PAGE_POOL_STATS
+	help
+	  This driver supports the gigabit ethernet MACs in the
+	  Airoha SoC family.
+
 config NET_MEDIATEK_SOC_WED
 	depends on ARCH_MEDIATEK || COMPILE_TEST
 	def_bool NET_MEDIATEK_SOC != n
drivers/net/ethernet/mediatek/Makefile +1 -0
index 03e008fbc859..ddbb7f4a516c 100644
@@ -11,3 +11,4 @@ mtk_eth-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_debugfs.o
 endif
 obj-$(CONFIG_NET_MEDIATEK_SOC_WED) += mtk_wed_ops.o
 obj-$(CONFIG_NET_MEDIATEK_STAR_EMAC) += mtk_star_emac.o
+obj-$(CONFIG_NET_AIROHA) += airoha_eth.o
drivers/net/ethernet/mediatek/airoha_eth.c new file +1783 -0

1,783 lines not on this page · raw message

drivers/net/ethernet/mediatek/airoha_eth.h new file +793 -0

793 lines not on this page · raw message

Signature · 1 line
2.45.2
#5 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 26 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +25 -0
+static int airoha_fe_set_pse_oq_rsv(struct airoha_eth *eth,
+				    u32 port, u32 queue, u32 val)
+{
+	u32 orig_val, tmp, all_rsv, fq_limit;
+	const u32 pse_port_oq_id[] = {
+		PSE_PORT0_QUEUE,
+		PSE_PORT1_QUEUE,
+		PSE_PORT2_QUEUE,
+		PSE_PORT3_QUEUE,
+		PSE_PORT4_QUEUE,
+		PSE_PORT5_QUEUE,
+		PSE_PORT6_QUEUE,
+		PSE_PORT7_QUEUE,
+		PSE_PORT8_QUEUE,
+		PSE_PORT9_QUEUE,
+		PSE_PORT10_QUEUE
+	};
 
+static void airoha_fe_oq_rsv_init(struct airoha_eth *eth)
+{
+	int i;
+
+	/* hw misses PPE2 oq rsv */
+	airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
+		      PSE_DEF_RSV_PAGE * PSE_PORT8_QUEUE);
+

drivers/net/ethernet/mediatek/airoha_eth.c line 340

Quoted 10 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +10 -0
+	for (i = 0; i < PSE_PORT0_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 0, i, 0x40);
+	for (i = 0; i < PSE_PORT1_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 1, i, 0x40);
+
+	for (i = 6; i < PSE_PORT2_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 2, i, 0);
+
+	for (i = 0; i < PSE_PORT3_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 3, i, 0x40);

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

If there some open architecture documentation for this device?

What are these ports about?

Quoted 1 line

+static void airoha_qdma_clenaup_rx_queue(struct airoha_queue *q)

cleanup?

drivers/net/ethernet/mediatek/airoha_eth.c line 1238

Quoted 9 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +9 -0
+static int airoha_dev_open(struct net_device *dev)
+{
+	struct airoha_eth *eth = netdev_priv(dev);
+	int err;
+
+	if (netdev_uses_dsa(dev))
+		airoha_fe_set(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);
+	else
+		airoha_fe_clear(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);

Does that imply both instances of the GMAC are not connected to the switch? Can one be used with a PHY?

Andrew

#6 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Hi,

Quoted 20 lines

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

If there some open architecture documentation for this device?

What are these ports about?

drivers/net/ethernet/mediatek/airoha_eth.c line 1238

Quoted 9 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +9 -0
+static int airoha_dev_open(struct net_device *dev)
+{
+	struct airoha_eth *eth = netdev_priv(dev);
+	int err;
+
+	if (netdev_uses_dsa(dev))
+		airoha_fe_set(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);
+	else
+		airoha_fe_clear(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);

Does that imply both instances of the GMAC are not connected to the switch? Can one be used with a PHY?

Quoted 1 line

Andrew

https://mirror2.openwrt.org/docs/MT7981B_Wi-Fi6_Platform_Datasheet_Open_V1.0.pdf

page 107 (text for 9.1.1 is relevant but not a complete match). In the EN7581 case there is a 5 port switch in the place of GMAC1 (one switch port is connected to GDM1).

This is documentation for another SoC but it should hopefully answer your questions.

MvH

Benjamin Larsson

#7 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 21 lines · Benjamin Larsson

Hi,

Quoted 20 lines

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

If there some open architecture documentation for this device?

What are these ports about?

drivers/net/ethernet/mediatek/airoha_eth.c line 1238

Quoted 9 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +9 -0
+static int airoha_dev_open(struct net_device *dev)
+{
+	struct airoha_eth *eth = netdev_priv(dev);
+	int err;
+
+	if (netdev_uses_dsa(dev))
+		airoha_fe_set(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);
+	else
+		airoha_fe_clear(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);

Does that imply both instances of the GMAC are not connected to the switch? Can one be used with a PHY?

Quoted 7 lines
Quoted 1 line

Andrew

https://mirror2.openwrt.org/docs/MT7981B_Wi-Fi6_Platform_Datasheet_Open_V1.0.pdf

page 107 (text for 9.1.1 is relevant but not a complete match). In the EN7581 case there is a 5 port switch in the place of GMAC1 (one switch port is connected to GDM1).

The typical DSA architecture is that the SoC MAC is connected to a switch MAC port. You say here, the switch is directly connected to the GGM1. So there is no GMAC involved? If there is no MAC, you don't need a MAC driver.

It seems more likely there is a GMAC, and the SGMII interface, or something similar is connected to the switch?

Andrew

#8 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 10 lines · Andrew Lunn
Quoted 5 lines

https://mirror2.openwrt.org/docs/MT7981B_Wi-Fi6_Platform_Datasheet_Open_V1.0.pdf

page 107 (text for 9.1.1 is relevant but not a complete match). In the EN7581 case there is a 5 port switch in the place of GMAC1 (one switch port is connected to GDM1).

The typical DSA architecture is that the SoC MAC is connected to a switch MAC port. You say here, the switch is directly connected to the GGM1. So there is no GMAC involved? If there is no MAC, you don't need a MAC driver.

Quoted 5 lines

It seems more likely there is a GMAC, and the SGMII interface, or something similar is connected to the switch?

Andrew

Hi, the Mediatek MT7988A SoC block diagram is similar to the EN7581 as it also have a built in switch (MT7531). MT7988A manual can be found here:

wiki.banana-pi.org/Banana_Pi_BPI-R4

drive.google.com/drive/folders/1XiVchy0a4syYFVlTndhVCETNJ9x7KOYi

MT7988A_Wi-Fi7_Platform_Datasheet_Open_V0.1.pdf

with page 125 containing the block diagram.

Per my understanding the MT7530 switch port connecting to the SoC in the EN7581 has a direct connection to the GDM1 port. There seem to not be any SGMII and MAC in this data path. How it actually is connected internally is unknown to me but packets seem to transfer automatically between the GDM1 port and the MT7530 switch soc port.

And as mentioned before there are high speed MACs capable of HSGMII/USXGMII connected to the other GDM ports.

So there will be MACs involved just not for the connection to the MT7530 switch.

MvH

Benjamin Larsson

author maintainer
#9 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

In reply to Andrew Lunn
Quoted 35 lines · Benjamin Larsson
Quoted 21 lines

Hi,

Quoted 20 lines

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

If there some open architecture documentation for this device?

What are these ports about?

drivers/net/ethernet/mediatek/airoha_eth.c line 1238

Quoted 9 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +9 -0
+static int airoha_dev_open(struct net_device *dev)
+{
+	struct airoha_eth *eth = netdev_priv(dev);
+	int err;
+
+	if (netdev_uses_dsa(dev))
+		airoha_fe_set(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);
+	else
+		airoha_fe_clear(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);

Does that imply both instances of the GMAC are not connected to the switch? Can one be used with a PHY?

Quoted 7 lines
Quoted 1 line

Andrew

https://mirror2.openwrt.org/docs/MT7981B_Wi-Fi6_Platform_Datasheet_Open_V1.0.pdf

page 107 (text for 9.1.1 is relevant but not a complete match). In the EN7581 case there is a 5 port switch in the place of GMAC1 (one switch port is connected to GDM1).

The typical DSA architecture is that the SoC MAC is connected to a switch MAC port. You say here, the switch is directly connected to the GGM1. So there is no GMAC involved? If there is no MAC, you don't need a MAC driver.

Quoted 5 lines

It seems more likely there is a GMAC, and the SGMII interface, or something similar is connected to the switch?

Andrew

The EN7581 architecture is similar to MT7988a one. There is a MAC port (GDM1) connected to a MT7530 DSA switch. With 'directly connected', I think Benjamin means we rely on the switch internal PHYs for GDM1. Moreover the SoC supports other MAC ports (GDM2, GDM3, GDM4) that can be connected to extanl PHYs. In v4 I will rework the driver adding the capability to plug even GDM{2,3,4}.

Regards,
Lorenzo

author maintainer
#10 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

In reply to Andrew Lunn
Quoted 32 lines
Quoted 32 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +31 -0
+static int airoha_fe_set_pse_oq_rsv(struct airoha_eth *eth,
+				    u32 port, u32 queue, u32 val)
+{
+	u32 orig_val, tmp, all_rsv, fq_limit;
+	const u32 pse_port_oq_id[] = {
+		PSE_PORT0_QUEUE,
+		PSE_PORT1_QUEUE,
+		PSE_PORT2_QUEUE,
+		PSE_PORT3_QUEUE,
+		PSE_PORT4_QUEUE,
+		PSE_PORT5_QUEUE,
+		PSE_PORT6_QUEUE,
+		PSE_PORT7_QUEUE,
+		PSE_PORT8_QUEUE,
+		PSE_PORT9_QUEUE,
+		PSE_PORT10_QUEUE
+	};
 
+static void airoha_fe_oq_rsv_init(struct airoha_eth *eth)
+{
+	int i;
+
+	/* hw misses PPE2 oq rsv */
+	airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
+		      PSE_DEF_RSV_PAGE * PSE_PORT8_QUEUE);
+
+	for (i = 0; i < PSE_PORT0_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 0, i, 0x40);
+	for (i = 0; i < PSE_PORT1_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 1, i, 0x40);
+
+	for (i = 6; i < PSE_PORT2_QUEUE; i++)
Quoted 8 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 340

Quoted 4 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +4 -0
+		airoha_fe_set_pse_oq_rsv(eth, 2, i, 0);
+
+	for (i = 0; i < PSE_PORT3_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 3, i, 0x40);

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

airoha_eth driver implements just MAC features (FE and QDMA). Currently we only support the connection to the DSA switch (GDM1). EN7581 SoC relies on mt7530 driver for DSA (I have not posted the patch for mt7530 yet, I will do after airoha_eth ones).

Quoted 4 lines

If there some open architecture documentation for this device?

What are these ports about?

airoha_fe_oq_rsv_init() (we can improve naming here :) is supposed to configure hw pre-allocated memory for each queue available in Packet Switching Engine (PSE) ports. PSE ports are not switch ports, but SoC internal ports used to connect PSE to different modules. In particular, we are currently implementing just the two connections below:
- CDM1 (port0) connects PSE to QDMA1
- GDM1 (port1) connects PSE to MT7530 DSA switch

In the future we will post support for GDM2, GDM3 and GDM4 ports that are connecting PSE to exteranl PHY modules.

Quoted 4 lines
Quoted 2 lines

+static void airoha_qdma_clenaup_rx_queue(struct airoha_queue *q)

cleanup?

ack, I will fix it.

Quoted 13 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 1238

Quoted 10 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +9 -0
+static int airoha_dev_open(struct net_device *dev)
+{
+	struct airoha_eth *eth = netdev_priv(dev);
+	int err;
+
+	if (netdev_uses_dsa(dev))
+		airoha_fe_set(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);
+	else
+		airoha_fe_clear(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);

Does that imply both instances of the GMAC are not connected to the switch? Can one be used with a PHY?

The check above is used to support configuration where MT7530 DSA switch module is not loaded (I tested this configuration removing the MT7530 DSA switch from board dts and resetting the switch). Since for the moment we just support GDM1 port (PSE port connected to the switch) we can probably assume it is always the case and remove this check. In the future we will need this configuration to support GDM2 or GDM3 (PSE port connected to external phy modules). Do you prefer to always set GDM1_STAG_EN_MASK for the moment?

Regards,
Lorenzo

Quoted 2 lines

Andrew

#11 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 41 lines
Quoted 14 lines
Quoted 14 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +14 -0
+static void airoha_fe_oq_rsv_init(struct airoha_eth *eth)
+{
+	int i;
+
+	/* hw misses PPE2 oq rsv */
+	airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
+		      PSE_DEF_RSV_PAGE * PSE_PORT8_QUEUE);
+
+	for (i = 0; i < PSE_PORT0_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 0, i, 0x40);
+	for (i = 0; i < PSE_PORT1_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 1, i, 0x40);
+
+	for (i = 6; i < PSE_PORT2_QUEUE; i++)
Quoted 8 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 340

Quoted 4 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +4 -0
+		airoha_fe_set_pse_oq_rsv(eth, 2, i, 0);
+
+	for (i = 0; i < PSE_PORT3_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 3, i, 0x40);

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

airoha_eth driver implements just MAC features (FE and QDMA). Currently we only support the connection to the DSA switch (GDM1). EN7581 SoC relies on mt7530 driver for DSA (I have not posted the patch for mt7530 yet, I will do after airoha_eth ones).

Quoted 4 lines

If there some open architecture documentation for this device?

What are these ports about?

airoha_fe_oq_rsv_init() (we can improve naming here :) is supposed to configure hw pre-allocated memory for each queue available in Packet Switching Engine (PSE) ports. PSE ports are not switch ports, but SoC internal ports used to connect PSE to different modules. In particular, we are currently implementing just the two connections below:
- CDM1 (port0) connects PSE to QDMA1
- GDM1 (port1) connects PSE to MT7530 DSA switch

Quoted 2 lines

In the future we will post support for GDM2, GDM3 and GDM4 ports that are connecting PSE to exteranl PHY modules.

I've not looked at the datasheet yet, but maybe add some ASCII art diagram of the architecture in the commit message, or even a .rst file somewhere under Documentation. It is hard to get the big picture looking at just the code, and only the MAC driver without all the other parts.

Quoted 13 lines
Quoted 4 lines
Quoted 4 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +4 -0
+static int airoha_dev_open(struct net_device *dev)
+{
+	struct airoha_eth *eth = netdev_priv(dev);
+	int err;
Quoted 8 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 1238

Quoted 5 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +5 -0
+
+	if (netdev_uses_dsa(dev))
+		airoha_fe_set(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);
+	else
+		airoha_fe_clear(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);

Does that imply both instances of the GMAC are not connected to the switch? Can one be used with a PHY?

Quoted 7 lines

The check above is used to support configuration where MT7530 DSA switch module is not loaded (I tested this configuration removing the MT7530 DSA switch from board dts and resetting the switch). Since for the moment we just support GDM1 port (PSE port connected to the switch) we can probably assume it is always the case and remove this check. In the future we will need this configuration to support GDM2 or GDM3 (PSE port connected to external phy modules). Do you prefer to always set GDM1_STAG_EN_MASK for the moment?

If it will be needed, then keep it. But it is the sort of thing which raises questions, so its good to explain it, either in the commit message, or in the code.

Andrew

author maintainer
#12 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 41 lines
Quoted 41 lines
Quoted 14 lines
Quoted 14 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +14 -0
+static void airoha_fe_oq_rsv_init(struct airoha_eth *eth)
+{
+	int i;
+
+	/* hw misses PPE2 oq rsv */
+	airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
+		      PSE_DEF_RSV_PAGE * PSE_PORT8_QUEUE);
+
+	for (i = 0; i < PSE_PORT0_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 0, i, 0x40);
+	for (i = 0; i < PSE_PORT1_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 1, i, 0x40);
+
+	for (i = 6; i < PSE_PORT2_QUEUE; i++)
Quoted 8 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 340

Quoted 4 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +4 -0
+		airoha_fe_set_pse_oq_rsv(eth, 2, i, 0);
+
+	for (i = 0; i < PSE_PORT3_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 3, i, 0x40);

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

airoha_eth driver implements just MAC features (FE and QDMA). Currently we only support the connection to the DSA switch (GDM1). EN7581 SoC relies on mt7530 driver for DSA (I have not posted the patch for mt7530 yet, I will do after airoha_eth ones).

Quoted 4 lines

If there some open architecture documentation for this device?

What are these ports about?

airoha_fe_oq_rsv_init() (we can improve naming here :) is supposed to configure hw pre-allocated memory for each queue available in Packet Switching Engine (PSE) ports. PSE ports are not switch ports, but SoC internal ports used to connect PSE to different modules. In particular, we are currently implementing just the two connections below:
- CDM1 (port0) connects PSE to QDMA1
- GDM1 (port1) connects PSE to MT7530 DSA switch

Quoted 8 lines
Quoted 2 lines

In the future we will post support for GDM2, GDM3 and GDM4 ports that are connecting PSE to exteranl PHY modules.

I've not looked at the datasheet yet, but maybe add some ASCII art diagram of the architecture in the commit message, or even a .rst file somewhere under Documentation. It is hard to get the big picture looking at just the code, and only the MAC driver without all the other parts.

ack, I will do my best :)

Quoted 22 lines
Quoted 14 lines
Quoted 13 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 1238

Quoted 10 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +9 -0
+static int airoha_dev_open(struct net_device *dev)
+{
+	struct airoha_eth *eth = netdev_priv(dev);
+	int err;
+
+	if (netdev_uses_dsa(dev))
+		airoha_fe_set(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);
+	else
+		airoha_fe_clear(eth, REG_GDM1_INGRESS_CFG, GDM1_STAG_EN_MASK);

Does that imply both instances of the GMAC are not connected to the switch? Can one be used with a PHY?

Quoted 7 lines

The check above is used to support configuration where MT7530 DSA switch module is not loaded (I tested this configuration removing the MT7530 DSA switch from board dts and resetting the switch). Since for the moment we just support GDM1 port (PSE port connected to the switch) we can probably assume it is always the case and remove this check. In the future we will need this configuration to support GDM2 or GDM3 (PSE port connected to external phy modules). Do you prefer to always set GDM1_STAG_EN_MASK for the moment?

Quoted 3 lines

If it will be needed, then keep it. But it is the sort of thing which raises questions, so its good to explain it, either in the commit message, or in the code.

ack, I will add it in v4

Regards,
Lorenzo

Quoted 2 lines

Andrew

#13 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

In reply to Lorenzo Bianconi
Quoted 54 lines · Lorenzo Bianconi
Quoted 32 lines
Quoted 32 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +31 -0
+static int airoha_fe_set_pse_oq_rsv(struct airoha_eth *eth,
+				    u32 port, u32 queue, u32 val)
+{
+	u32 orig_val, tmp, all_rsv, fq_limit;
+	const u32 pse_port_oq_id[] = {
+		PSE_PORT0_QUEUE,
+		PSE_PORT1_QUEUE,
+		PSE_PORT2_QUEUE,
+		PSE_PORT3_QUEUE,
+		PSE_PORT4_QUEUE,
+		PSE_PORT5_QUEUE,
+		PSE_PORT6_QUEUE,
+		PSE_PORT7_QUEUE,
+		PSE_PORT8_QUEUE,
+		PSE_PORT9_QUEUE,
+		PSE_PORT10_QUEUE
+	};
 
+static void airoha_fe_oq_rsv_init(struct airoha_eth *eth)
+{
+	int i;
+
+	/* hw misses PPE2 oq rsv */
+	airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
+		      PSE_DEF_RSV_PAGE * PSE_PORT8_QUEUE);
+
+	for (i = 0; i < PSE_PORT0_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 0, i, 0x40);
+	for (i = 0; i < PSE_PORT1_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 1, i, 0x40);
+
+	for (i = 6; i < PSE_PORT2_QUEUE; i++)
Quoted 8 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 340

Quoted 4 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +4 -0
+		airoha_fe_set_pse_oq_rsv(eth, 2, i, 0);
+
+	for (i = 0; i < PSE_PORT3_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 3, i, 0x40);

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

airoha_eth driver implements just MAC features (FE and QDMA). Currently we only support the connection to the DSA switch (GDM1). EN7581 SoC relies on mt7530 driver for DSA (I have not posted the patch for mt7530 yet, I will do after airoha_eth ones).

airoha_fe_oq_rsv_init() (we can improve naming here :) is supposed to configure hw pre-allocated memory for each queue available in Packet Switching Engine (PSE) ports. PSE ports are not switch ports, but SoC internal ports used to connect PSE to different modules. In particular, we are currently implementing just the two connections below:
- CDM1 (port0) connects PSE to QDMA1
- GDM1 (port1) connects PSE to MT7530 DSA switch

Quoted 2 lines

In the future we will post support for GDM2, GDM3 and GDM4 ports that are connecting PSE to exteranl PHY modules.

Is the PSE involved in WiFi? When you come to implement NAT offload, etc, will that depend on the PSE?

Figure 9-1 of MT7981B_Wi-Fi6_Platform_Datasheet_Open_V1.0.pdf clearly shows the PSE outside of the GMAC. I'm just wondering if the PSE should be a driver, or library, of its own, which is then shared by users, rather than being embedded in the MAC driver?

Andrew

author maintainer
#14 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 55 lines · Lorenzo Bianconi
Quoted 54 lines
Quoted 32 lines
Quoted 32 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +31 -0
+static int airoha_fe_set_pse_oq_rsv(struct airoha_eth *eth,
+				    u32 port, u32 queue, u32 val)
+{
+	u32 orig_val, tmp, all_rsv, fq_limit;
+	const u32 pse_port_oq_id[] = {
+		PSE_PORT0_QUEUE,
+		PSE_PORT1_QUEUE,
+		PSE_PORT2_QUEUE,
+		PSE_PORT3_QUEUE,
+		PSE_PORT4_QUEUE,
+		PSE_PORT5_QUEUE,
+		PSE_PORT6_QUEUE,
+		PSE_PORT7_QUEUE,
+		PSE_PORT8_QUEUE,
+		PSE_PORT9_QUEUE,
+		PSE_PORT10_QUEUE
+	};
 
+static void airoha_fe_oq_rsv_init(struct airoha_eth *eth)
+{
+	int i;
+
+	/* hw misses PPE2 oq rsv */
+	airoha_fe_set(eth, REG_FE_PSE_BUF_SET,
+		      PSE_DEF_RSV_PAGE * PSE_PORT8_QUEUE);
+
+	for (i = 0; i < PSE_PORT0_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 0, i, 0x40);
+	for (i = 0; i < PSE_PORT1_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 1, i, 0x40);
+
+	for (i = 6; i < PSE_PORT2_QUEUE; i++)
Quoted 8 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 340

Quoted 4 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +4 -0
+		airoha_fe_set_pse_oq_rsv(eth, 2, i, 0);
+
+	for (i = 0; i < PSE_PORT3_QUEUE; i++)
+		airoha_fe_set_pse_oq_rsv(eth, 3, i, 0x40);

Code like this is making me wounder about the split between MAC driver, DSA driver and DSA tag driver. Or if it should actually be a pure switchdev driver?

airoha_eth driver implements just MAC features (FE and QDMA). Currently we only support the connection to the DSA switch (GDM1). EN7581 SoC relies on mt7530 driver for DSA (I have not posted the patch for mt7530 yet, I will do after airoha_eth ones).

airoha_fe_oq_rsv_init() (we can improve naming here :) is supposed to configure hw pre-allocated memory for each queue available in Packet Switching Engine (PSE) ports. PSE ports are not switch ports, but SoC internal ports used to connect PSE to different modules. In particular, we are currently implementing just the two connections below:
- CDM1 (port0) connects PSE to QDMA1
- GDM1 (port1) connects PSE to MT7530 DSA switch

Quoted 6 lines
Quoted 2 lines

In the future we will post support for GDM2, GDM3 and GDM4 ports that are connecting PSE to exteranl PHY modules.

Is the PSE involved in WiFi? When you come to implement NAT offload, etc, will that depend on the PSE?

Frame Engine architecture (Packet Switching Engine - PSE + Packet Processing Engine - PPE) is similar to mtk_eth_soc driver (but register map and QDMA layer are different). FE is used to define offload traffic rules (e.g. forwarding between GDM interfaces or NAT rules). Even in mtk_eth_soc frame engine codebase is part of MAC driver. So far we do not support offloading rules, but we will do in the future. In order to support WiFI offload rules, FE requires a different module/driver (NPU) similar to WED for mtk_eth_soc.
The mac - switch architecture is the same to the one used for MT7988a [0]. So far we support we support just gmac0 (that is connected to the switch). In the future I will add support for gmac1 as well.

Regards,
Lorenzo

[0] https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/mediatek/files-6.6/arch/arm64/boot/dts/mediatek/mt7988a.dtsi;h=9ad068fe05fc52bf1edc28d7dba4e162f30a0eb8;hb=HEAD

Quoted 8 lines

Figure 9-1 of MT7981B_Wi-Fi6_Platform_Datasheet_Open_V1.0.pdf clearly shows the PSE outside of the GMAC. I'm just wondering if the PSE should be a driver, or library, of its own, which is then shared by users, rather than being embedded in the MAC driver?

Andrew

#15 lore raw

RE: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

In reply to Lorenzo Bianconi
Quoted 19 lines

Add airoha_eth driver in order to introduce ethernet support for Airoha EN7581 SoC available on EN7581 development board (en7581-evb). en7581-evb networking architecture is composed by airoha_eth as mac controller (cpu port) and a mt7530 dsa based switch.
EN7581 mac controller is mainly composed by Frame Engine (FE) and QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic functionalities are supported now) while QDMA is used for DMA operation and QOS functionalities between mac layer and the dsa switch (hw QoS is not available yet and it will be added in the future). Currently only hw lan features are available, hw wan will be added with subsequent patches.

Tested-by Benjamin Larsson <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

---

drivers/net/ethernet/mediatek/airoha_eth.c new file +3 -0
+
+static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
+{

......................

drivers/net/ethernet/mediatek/airoha_eth.c line 639

Quoted 1 line
drivers/net/ethernet/mediatek/airoha_eth.c new file +1 -0
+		skb->ip_summed = CHECKSUM_UNNECESSARY;

Unconditionally setting UNNECESSARY for all pkts.
Does HW reports csum errors ?

Thanks,
Sunil.

author maintainer
#16 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 16 lines · Sunil Kovvuri Goutham
Quoted 16 lines

Add airoha_eth driver in order to introduce ethernet support for Airoha EN7581 SoC available on EN7581 development board (en7581-evb). en7581-evb networking architecture is composed by airoha_eth as mac controller (cpu port) and a mt7530 dsa based switch.
EN7581 mac controller is mainly composed by Frame Engine (FE) and QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic functionalities are supported now) while QDMA is used for DMA operation and QOS functionalities between mac layer and the dsa switch (hw QoS is not available yet and it will be added in the future). Currently only hw lan features are available, hw wan will be added with subsequent patches.

Tested-by Benjamin Larsson <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

---

Quoted 8 lines
Quoted 3 lines
drivers/net/ethernet/mediatek/airoha_eth.c new file +3 -0
+
+static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
+{

......................

drivers/net/ethernet/mediatek/airoha_eth.c line 639

Quoted 1 line
drivers/net/ethernet/mediatek/airoha_eth.c new file +1 -0
+		skb->ip_summed = CHECKSUM_UNNECESSARY;

Unconditionally setting UNNECESSARY for all pkts.
Does HW reports csum errors ?

We enabled packet drop if TCP/UDP or IPv4 checksum is failed in REG_GDM1_FWD_CFG register in airoha_fe_maccr_init(). In other words, if the nic forwards the packet to the cpu, the checksum is correct.

Regards,
Lorenzo

Quoted 4 lines

Thanks,
Sunil.

reviewer
#17 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

In reply to Lorenzo Bianconi
Quoted 12 lines · Lorenzo Bianconi

Add airoha_eth driver in order to introduce ethernet support for Airoha EN7581 SoC available on EN7581 development board (en7581-evb). en7581-evb networking architecture is composed by airoha_eth as mac controller (cpu port) and a mt7530 dsa based switch.
EN7581 mac controller is mainly composed by Frame Engine (FE) and QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic functionalities are supported now) while QDMA is used for DMA operation and QOS functionalities between mac layer and the dsa switch (hw QoS is not available yet and it will be added in the future). Currently only hw lan features are available, hw wan will be added with subsequent patches.

Quoted 2 lines
Tested-by Benjamin Larsson <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

Hi Lorenzo,

Some minor nits from my side.

...

Quoted 1 line
drivers/net/ethernet/mediatek/airoha_eth.c +0 -0

...

drivers/net/ethernet/mediatek/airoha_eth.c line 49

Quoted 11 lines
drivers/net/ethernet/mediatek/airoha_eth.c +11 -0
+#define airoha_fe_rr(eth, offset)		airoha_rr((eth)->fe_regs, (offset))
+#define airoha_fe_wr(eth, offset, val)		airoha_wr((eth)->fe_regs, (offset), (val))
+#define airoha_fe_rmw(eth, offset, mask, val)	airoha_rmw((eth)->fe_regs, (offset), (mask), (val))
+#define airoha_fe_set(eth, offset, val)		airoha_rmw((eth)->fe_regs, (offset), 0, (val))
+#define airoha_fe_clear(eth, offset, val)	airoha_rmw((eth)->fe_regs, (offset), (val), 0)
+
+#define airoha_qdma_rr(eth, offset)		airoha_rr((eth)->qdma_regs, (offset))
+#define airoha_qdma_wr(eth, offset, val)	airoha_wr((eth)->qdma_regs, (offset), (val))
+#define airoha_qdma_rmw(eth, offset, mask, val)	airoha_rmw((eth)->qdma_regs, (offset), (mask), (val))
+#define airoha_qdma_set(eth, offset, val)	airoha_rmw((eth)->qdma_regs, (offset), 0, (val))
+#define airoha_qdma_clear(eth, offset, val)	airoha_rmw((eth)->qdma_regs, (offset), (val), 0)

nit: Please consider line-wrapping the above so lines are 80 columns wide
or less, which is still preferred in Networking code.

Flagged by checkpatch.pl --max-line-length=80

...

Quoted 15 lines
drivers/net/ethernet/mediatek/airoha_eth.c +15 -0
+static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
+				   struct net_device *dev)
+{
+	struct skb_shared_info *sinfo = skb_shinfo(skb);
+	struct airoha_eth *eth = netdev_priv(dev);
+	int i, qid = skb_get_queue_mapping(skb);
+	u32 nr_frags, msg0 = 0, msg1;
+	u32 len = skb_headlen(skb);
+	struct netdev_queue *txq;
+	struct airoha_queue *q;
+	void *data = skb->data;
+	u16 index;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		msg0 |= FIELD_PREP(QDMA_ETH_TXMSG_TCO_MASK, 1) |

drivers/net/ethernet/mediatek/airoha_eth.c line 1314

Quoted 10 lines
drivers/net/ethernet/mediatek/airoha_eth.c +10 -0
+			FIELD_PREP(QDMA_ETH_TXMSG_UCO_MASK, 1) |
+			FIELD_PREP(QDMA_ETH_TXMSG_ICO_MASK, 1);
+
+	/* TSO: fill MSS info in tcp checksum field */
+	if (skb_is_gso(skb)) {
+		if (skb_cow_head(skb, 0))
+			goto error;
+
+		if (sinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) {
+			tcp_hdr(skb)->check = cpu_to_be16(sinfo->gso_size);

Probably we could do better with an appropriate helper - perhaps there is one I couldn't find one - but I think you need a cast here to keep Sparse happy.

Something like this (completely untested!):

			tcp_hdr(skb)->check =
				(__force __sum16)cpu_to_be16(sinfo->gso_size);

...

Quoted 51 lines
drivers/net/ethernet/mediatek/airoha_eth.c +51 -0
+static int airoha_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct net_device *dev;
+	struct airoha_eth *eth;
+	int err;
+
+	dev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(*eth),
+				      AIROHA_NUM_TX_RING, AIROHA_NUM_RX_RING);
+	if (!dev) {
+		dev_err(&pdev->dev, "alloc_etherdev failed\n");
+		return -ENOMEM;
+	}
+
+	eth = netdev_priv(dev);
+	eth->net_dev = dev;
+
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (err) {
+		dev_err(&pdev->dev, "failed configuring DMA mask\n");
+		return err;
+	}
+
+	eth->fe_regs = devm_platform_ioremap_resource_byname(pdev, "fe");
+	if (IS_ERR(eth->fe_regs))
+		return dev_err_probe(&pdev->dev, PTR_ERR(eth->fe_regs),
+				     "failed to iomap fe regs\n");
+
+	eth->qdma_regs = devm_platform_ioremap_resource_byname(pdev, "qdma0");
+	if (IS_ERR(eth->qdma_regs))
+		return dev_err_probe(&pdev->dev, PTR_ERR(eth->qdma_regs),
+				     "failed to iomap qdma regs\n");
+
+	eth->rsts[0].id = "fe";
+	eth->rsts[1].id = "pdma";
+	eth->rsts[2].id = "qdma";
+	err = devm_reset_control_bulk_get_exclusive(&pdev->dev,
+						    ARRAY_SIZE(eth->rsts),
+						    eth->rsts);
+	if (err) {
+		dev_err(&pdev->dev, "failed to get bulk reset lines\n");
+		return err;
+	}
+
+	eth->xsi_rsts[0].id = "xsi-mac";
+	eth->xsi_rsts[1].id = "hsi0-mac";
+	eth->xsi_rsts[2].id = "hsi1-mac";
+	eth->xsi_rsts[3].id = "hsi-mac";
+	eth->xsi_rsts[4].id = "xfp-mac";
+	err = devm_reset_control_bulk_get_exclusive(&pdev->dev,
+						    ARRAY_SIZE(eth->xsi_rsts),

drivers/net/ethernet/mediatek/airoha_eth.c line 1698

Quoted 10 lines
drivers/net/ethernet/mediatek/airoha_eth.c +10 -0
+						    eth->xsi_rsts);
+	if (err) {
+		dev_err(&pdev->dev, "failed to get bulk xsi reset lines\n");
+		return err;
+	}
+
+	spin_lock_init(&eth->irq_lock);
+	eth->irq = platform_get_irq(pdev, 0);
+	if (eth->irq < 0) {
+		dev_err(&pdev->dev, "failed reading irq line\n");

Coccinelle says:

.../airoha_eth.c:1698:2-9: line 1698 is redundant because platform_get_irq() already prints an error

...

drivers/net/ethernet/mediatek/airoha_eth.c line 1769

Quoted 4 lines
drivers/net/ethernet/mediatek/airoha_eth.c +4 -0
+const struct of_device_id of_airoha_match[] = {
+	{ .compatible = "airoha,en7581-eth" },
+	{ /* sentinel */ }
+};

of_airoha_match appears to only be used in this file. If so, it should be static.

Flagged by Sparse.

Quoted 21 lines
drivers/net/ethernet/mediatek/airoha_eth.c +14 -0
+
+static struct platform_driver airoha_driver = {
+	.probe = airoha_probe,
+	.remove_new = airoha_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = of_airoha_match,
+	},
+};
+module_platform_driver(airoha_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
+MODULE_DESCRIPTION("Ethernet driver for Airoha SoC");
drivers/net/ethernet/mediatek/airoha_eth.h new file +0 -0
new file mode 100644
index 000000000000..f7b984be4d60

drivers/net/ethernet/mediatek/airoha_eth.h line 1

Quoted 1 line
drivers/net/ethernet/mediatek/airoha_eth.h new file +1 -0
@@ -0,0 +1,793 @@
+// SPDX-License-Identifier: GPL-2.0

The correct SPDX header comment style for .h (but not .c) files is /* ... */

https://docs.kernel.org/6.9/process/license-rules.html#license-identifier-syntax

Flagged by checkpatch

...

author maintainer
#18 lore raw

Re: [PATCH v3 net-next 2/2] net: airoha: Introduce ethernet support for EN7581 SoC

Quoted 13 lines · Lorenzo Bianconi
Quoted 12 lines

Add airoha_eth driver in order to introduce ethernet support for Airoha EN7581 SoC available on EN7581 development board (en7581-evb). en7581-evb networking architecture is composed by airoha_eth as mac controller (cpu port) and a mt7530 dsa based switch.
EN7581 mac controller is mainly composed by Frame Engine (FE) and QoS-DMA (QDMA) modules. FE is used for traffic offloading (just basic functionalities are supported now) while QDMA is used for DMA operation and QOS functionalities between mac layer and the dsa switch (hw QoS is not available yet and it will be added in the future). Currently only hw lan features are available, hw wan will be added with subsequent patches.

Quoted 6 lines
Quoted 2 lines
Tested-by Benjamin Larsson <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

Hi Lorenzo,

Some minor nits from my side.

Hi Simon,

thx for the review.

Quoted 22 lines

...

Quoted 1 line
drivers/net/ethernet/mediatek/airoha_eth.c +0 -0

...

drivers/net/ethernet/mediatek/airoha_eth.c line 49

Quoted 11 lines
drivers/net/ethernet/mediatek/airoha_eth.c +11 -0
+#define airoha_fe_rr(eth, offset)		airoha_rr((eth)->fe_regs, (offset))
+#define airoha_fe_wr(eth, offset, val)		airoha_wr((eth)->fe_regs, (offset), (val))
+#define airoha_fe_rmw(eth, offset, mask, val)	airoha_rmw((eth)->fe_regs, (offset), (mask), (val))
+#define airoha_fe_set(eth, offset, val)		airoha_rmw((eth)->fe_regs, (offset), 0, (val))
+#define airoha_fe_clear(eth, offset, val)	airoha_rmw((eth)->fe_regs, (offset), (val), 0)
+
+#define airoha_qdma_rr(eth, offset)		airoha_rr((eth)->qdma_regs, (offset))
+#define airoha_qdma_wr(eth, offset, val)	airoha_wr((eth)->qdma_regs, (offset), (val))
+#define airoha_qdma_rmw(eth, offset, mask, val)	airoha_rmw((eth)->qdma_regs, (offset), (mask), (val))
+#define airoha_qdma_set(eth, offset, val)	airoha_rmw((eth)->qdma_regs, (offset), 0, (val))
+#define airoha_qdma_clear(eth, offset, val)	airoha_rmw((eth)->qdma_regs, (offset), (val), 0)

nit: Please consider line-wrapping the above so lines are 80 columns wide
or less, which is still preferred in Networking code.

Quoted 1 line

Flagged by checkpatch.pl --max-line-length=80

ack, I will fix it in v4.

Quoted 33 lines

...

Quoted 15 lines
drivers/net/ethernet/mediatek/airoha_eth.c +15 -0
+static netdev_tx_t airoha_dev_xmit(struct sk_buff *skb,
+				   struct net_device *dev)
+{
+	struct skb_shared_info *sinfo = skb_shinfo(skb);
+	struct airoha_eth *eth = netdev_priv(dev);
+	int i, qid = skb_get_queue_mapping(skb);
+	u32 nr_frags, msg0 = 0, msg1;
+	u32 len = skb_headlen(skb);
+	struct netdev_queue *txq;
+	struct airoha_queue *q;
+	void *data = skb->data;
+	u16 index;
+
+	if (skb->ip_summed == CHECKSUM_PARTIAL)
+		msg0 |= FIELD_PREP(QDMA_ETH_TXMSG_TCO_MASK, 1) |

drivers/net/ethernet/mediatek/airoha_eth.c line 1314

Quoted 10 lines
drivers/net/ethernet/mediatek/airoha_eth.c +10 -0
+			FIELD_PREP(QDMA_ETH_TXMSG_UCO_MASK, 1) |
+			FIELD_PREP(QDMA_ETH_TXMSG_ICO_MASK, 1);
+
+	/* TSO: fill MSS info in tcp checksum field */
+	if (skb_is_gso(skb)) {
+		if (skb_cow_head(skb, 0))
+			goto error;
+
+		if (sinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6)) {
+			tcp_hdr(skb)->check = cpu_to_be16(sinfo->gso_size);

Probably we could do better with an appropriate helper - perhaps there is one I couldn't find one - but I think you need a cast here to keep Sparse happy.

Quoted 6 lines

Something like this (completely untested!):

			tcp_hdr(skb)->check =
				(__force __sum16)cpu_to_be16(sinfo->gso_size);

...

ack, I will fix it in v4.

Quoted 65 lines
Quoted 52 lines
drivers/net/ethernet/mediatek/airoha_eth.c +51 -0
+static int airoha_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct net_device *dev;
+	struct airoha_eth *eth;
+	int err;
+
+	dev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(*eth),
+				      AIROHA_NUM_TX_RING, AIROHA_NUM_RX_RING);
+	if (!dev) {
+		dev_err(&pdev->dev, "alloc_etherdev failed\n");
+		return -ENOMEM;
+	}
+
+	eth = netdev_priv(dev);
+	eth->net_dev = dev;
+
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (err) {
+		dev_err(&pdev->dev, "failed configuring DMA mask\n");
+		return err;
+	}
+
+	eth->fe_regs = devm_platform_ioremap_resource_byname(pdev, "fe");
+	if (IS_ERR(eth->fe_regs))
+		return dev_err_probe(&pdev->dev, PTR_ERR(eth->fe_regs),
+				     "failed to iomap fe regs\n");
+
+	eth->qdma_regs = devm_platform_ioremap_resource_byname(pdev, "qdma0");
+	if (IS_ERR(eth->qdma_regs))
+		return dev_err_probe(&pdev->dev, PTR_ERR(eth->qdma_regs),
+				     "failed to iomap qdma regs\n");
+
+	eth->rsts[0].id = "fe";
+	eth->rsts[1].id = "pdma";
+	eth->rsts[2].id = "qdma";
+	err = devm_reset_control_bulk_get_exclusive(&pdev->dev,
+						    ARRAY_SIZE(eth->rsts),
+						    eth->rsts);
+	if (err) {
+		dev_err(&pdev->dev, "failed to get bulk reset lines\n");
+		return err;
+	}
+
+	eth->xsi_rsts[0].id = "xsi-mac";
+	eth->xsi_rsts[1].id = "hsi0-mac";
+	eth->xsi_rsts[2].id = "hsi1-mac";
+	eth->xsi_rsts[3].id = "hsi-mac";
+	eth->xsi_rsts[4].id = "xfp-mac";
+	err = devm_reset_control_bulk_get_exclusive(&pdev->dev,
+						    ARRAY_SIZE(eth->xsi_rsts),

drivers/net/ethernet/mediatek/airoha_eth.c line 1698

Quoted 10 lines
drivers/net/ethernet/mediatek/airoha_eth.c +10 -0
+						    eth->xsi_rsts);
+	if (err) {
+		dev_err(&pdev->dev, "failed to get bulk xsi reset lines\n");
+		return err;
+	}
+
+	spin_lock_init(&eth->irq_lock);
+	eth->irq = platform_get_irq(pdev, 0);
+	if (eth->irq < 0) {
+		dev_err(&pdev->dev, "failed reading irq line\n");

Coccinelle says:

Quoted 3 lines

.../airoha_eth.c:1698:2-9: line 1698 is redundant because platform_get_irq() already prints an error

...

ack, I will fix it in v4.

Quoted 10 lines

drivers/net/ethernet/mediatek/airoha_eth.c line 1769

Quoted 5 lines
drivers/net/ethernet/mediatek/airoha_eth.c +4 -0
+const struct of_device_id of_airoha_match[] = {
+	{ .compatible = "airoha,en7581-eth" },
+	{ /* sentinel */ }
+};

of_airoha_match appears to only be used in this file. If so, it should be static.

Flagged by Sparse.

ack, I will fix it in v4.

Quoted 26 lines
Quoted 22 lines
drivers/net/ethernet/mediatek/airoha_eth.c +14 -0
+
+static struct platform_driver airoha_driver = {
+	.probe = airoha_probe,
+	.remove_new = airoha_remove,
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = of_airoha_match,
+	},
+};
+module_platform_driver(airoha_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Lorenzo Bianconi <[email protected]>");
+MODULE_DESCRIPTION("Ethernet driver for Airoha SoC");
drivers/net/ethernet/mediatek/airoha_eth.h new file +0 -0
new file mode 100644
index 000000000000..f7b984be4d60

drivers/net/ethernet/mediatek/airoha_eth.h line 1

Quoted 1 line
drivers/net/ethernet/mediatek/airoha_eth.h new file +1 -0
@@ -0,0 +1,793 @@
+// SPDX-License-Identifier: GPL-2.0

The correct SPDX header comment style for .h (but not .c) files is /* ... */

Quoted 3 lines

https://docs.kernel.org/6.9/process/license-rules.html#license-identifier-syntax

Flagged by checkpatch

ack, I will fix it in v4.

Regards,
Lorenzo

Quoted 2 lines

...