This is a patch series of ethernet driver for Sunplus SP7021 SoC.

Add Ethernet driver for Sunplus SP7021.

Signed-off-by Wells Lu <[email protected]>

---
  MAINTAINERS                                  |   1 +
  drivers/net/ethernet/Kconfig                 |   1 +
  drivers/net/ethernet/Makefile                |   1 +
  drivers/net/ethernet/sunplus/Kconfig         |  20 +
  drivers/net/ethernet/sunplus/Makefile        |   6 +
  drivers/net/ethernet/sunplus/l2sw_define.h   | 221 ++++++++
  drivers/net/ethernet/sunplus/l2sw_desc.c     | 233 ++++++++
  drivers/net/ethernet/sunplus/l2sw_desc.h     |  21 +
  drivers/net/ethernet/sunplus/l2sw_driver.c   | 779 +++++++++++++++++++++++++++
  drivers/net/ethernet/sunplus/l2sw_driver.h   |  23 +
  drivers/net/ethernet/sunplus/l2sw_hal.c      | 422 +++++++++++++++
  drivers/net/ethernet/sunplus/l2sw_hal.h      |  47 ++
  drivers/net/ethernet/sunplus/l2sw_int.c      | 326 +++++++++++
  drivers/net/ethernet/sunplus/l2sw_int.h      |  16 +
  drivers/net/ethernet/sunplus/l2sw_mac.c      |  68 +++
  drivers/net/ethernet/sunplus/l2sw_mac.h      |  24 +
  drivers/net/ethernet/sunplus/l2sw_mdio.c     | 118 ++++
  drivers/net/ethernet/sunplus/l2sw_mdio.h     |  19 +
  drivers/net/ethernet/sunplus/l2sw_register.h |  99 ++++
  19 files changed, 2445 insertions(+)

create mode 100644 drivers/net/ethernet/sunplus/Kconfig create mode 100644 drivers/net/ethernet/sunplus/Makefile create mode 100644 drivers/net/ethernet/sunplus/l2sw_define.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_desc.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_desc.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_driver.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_driver.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_hal.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_hal.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_int.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_int.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_mac.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_mac.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_mdio.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_mdio.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_register.h

MAINTAINERS +1 -0

index 4669c16..ca676ec 100644
@@ -18006,6 +18006,7 @@ L:	[email protected]
  S:	Maintained
  W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
  F:	Documentation/devicetree/bindings/net/sunplus,sp7021-l2sw.yaml
+F:	drivers/net/ethernet/sunplus/
 
  SUPERH
  M:	Yoshinori Sato <[email protected]>

drivers/net/ethernet/Kconfig +1 -0

index 412ae3e..0084852 100644
@@ -176,6 +176,7 @@ source "drivers/net/ethernet/smsc/Kconfig"
  source "drivers/net/ethernet/socionext/Kconfig"
  source "drivers/net/ethernet/stmicro/Kconfig"
  source "drivers/net/ethernet/sun/Kconfig"
+source "drivers/net/ethernet/sunplus/Kconfig"
  source "drivers/net/ethernet/synopsys/Kconfig"
  source "drivers/net/ethernet/tehuti/Kconfig"
  source "drivers/net/ethernet/ti/Kconfig"

drivers/net/ethernet/Makefile +1 -0

index aaa5078..e4ce162 100644
@@ -87,6 +87,7 @@ obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
  obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
  obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
  obj-$(CONFIG_NET_VENDOR_SUN) += sun/
+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sunplus/
  obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
  obj-$(CONFIG_NET_VENDOR_TI) += ti/
  obj-$(CONFIG_NET_VENDOR_TOSHIBA) += toshiba/

drivers/net/ethernet/sunplus/Kconfig new file +20 -0

new file mode 100644
index 0000000..a9e3a4c
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Sunplus Ethernet device configuration
+#
+
+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
+	depends on ETHERNET && SOC_SP7021
+	select PHYLIB
+	select PINCTRL_SPPCTL
+	select COMMON_CLK_SP7021
+	select RESET_SUNPLUS
+	select NVMEM_SUNPLUS_OCOTP
+	help
+	  If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
+	  devices, say Y.
+	  The network device supports dual 10M/100M Ethernet interfaces,
+	  or one 10/100M Ethernet interface with two LAN ports.
+	  To compile this driver as a module, choose M here.  The module
+	  will be called sp_l2sw.

drivers/net/ethernet/sunplus/Makefile new file +6 -0

new file mode 100644
index 0000000..b401cec
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the Sunplus network device drivers.
+#
+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sp_l2sw.o
+sp_l2sw-objs := l2sw_driver.o l2sw_int.o l2sw_hal.o l2sw_desc.o l2sw_mac.o l2sw_mdio.o

drivers/net/ethernet/sunplus/l2sw_define.h new file +185 -0

new file mode 100644
index 0000000..c1049c5
@@ -0,0 +1,221 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+
+#ifndef __L2SW_DEFINE_H__
+#define __L2SW_DEFINE_H__
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/kdev_t.h>
+#include <linux/in.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+#include <linux/skbuff.h>
+#include <linux/ethtool.h>
+#include <linux/platform_device.h>
+#include <linux/phy.h>
+#include <linux/mii.h>
+#include <linux/if_vlan.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt)     "[L2SW]" fmt
+
+//define MAC interrupt status bit
+#define MAC_INT_DAISY_MODE_CHG          BIT(31)
+#define MAC_INT_IP_CHKSUM_ERR           BIT(23)
+#define MAC_INT_WDOG_TIMER1_EXP         BIT(22)
+#define MAC_INT_WDOG_TIMER0_EXP         BIT(21)
+#define MAC_INT_INTRUDER_ALERT          BIT(20)
+#define MAC_INT_PORT_ST_CHG             BIT(19)
+#define MAC_INT_BC_STORM                BIT(18)
+#define MAC_INT_MUST_DROP_LAN           BIT(17)
+#define MAC_INT_GLOBAL_QUE_FULL         BIT(16)
+#define MAC_INT_TX_SOC_PAUSE_ON         BIT(15)
+#define MAC_INT_RX_SOC_QUE_FULL         BIT(14)
+#define MAC_INT_TX_LAN1_QUE_FULL        BIT(9)
+#define MAC_INT_TX_LAN0_QUE_FULL        BIT(8)
+#define MAC_INT_RX_L_DESCF              BIT(7)
+#define MAC_INT_RX_H_DESCF              BIT(6)
+#define MAC_INT_RX_DONE_L               BIT(5)
+#define MAC_INT_RX_DONE_H               BIT(4)
+#define MAC_INT_TX_DONE_L               BIT(3)
+#define MAC_INT_TX_DONE_H               BIT(2)
+#define MAC_INT_TX_DES_ERR              BIT(1)
+#define MAC_INT_RX_DES_ERR              BIT(0)
+
+#define MAC_INT_RX                      (MAC_INT_RX_DONE_H | MAC_INT_RX_DONE_L | \
+					MAC_INT_RX_DES_ERR)
+#define MAC_INT_TX                      (MAC_INT_TX_DONE_L | MAC_INT_TX_DONE_H | \
+					MAC_INT_TX_DES_ERR)
+#define MAC_INT_MASK_DEF                (MAC_INT_DAISY_MODE_CHG | MAC_INT_IP_CHKSUM_ERR | \
+					MAC_INT_WDOG_TIMER1_EXP | MAC_INT_WDOG_TIMER0_EXP | \
+					MAC_INT_INTRUDER_ALERT | MAC_INT_BC_STORM | \
+					MAC_INT_MUST_DROP_LAN | MAC_INT_GLOBAL_QUE_FULL | \
+					MAC_INT_TX_SOC_PAUSE_ON | MAC_INT_RX_SOC_QUE_FULL | \
+					MAC_INT_TX_LAN1_QUE_FULL | MAC_INT_TX_LAN0_QUE_FULL | \
+					MAC_INT_RX_L_DESCF | MAC_INT_RX_H_DESCF)
+
+/*define port ability*/
+#define PORT_ABILITY_LINK_ST_P1         BIT(25)
+#define PORT_ABILITY_LINK_ST_P0         BIT(24)
+
+/*define PHY command bit*/
+#define PHY_WT_DATA_MASK                0xffff0000
+#define PHY_RD_CMD                      0x00004000
+#define PHY_WT_CMD                      0x00002000
+#define PHY_REG_MASK                    0x00001f00
+#define PHY_ADR_MASK                    0x0000001f
+
+/*define PHY status bit*/
+#define PHY_RD_DATA_MASK                0xffff0000
+#define PHY_RD_RDY                      BIT(1)
+#define PHY_WT_DONE                     BIT(0)
+
+/*define other register bit*/
+#define RX_MAX_LEN_MASK                 0x00011000
+#define ROUTE_MODE_MASK                 0x00000060
+#define POK_INT_THS_MASK                0x000E0000
+#define VLAN_TH_MASK                    0x00000007
+
+/*define tx descriptor bit*/
+#define OWN_BIT                         BIT(31)
+#define FS_BIT                          BIT(25)
+#define LS_BIT                          BIT(24)
+#define LEN_MASK                        0x000007FF
+#define PKTSP_MASK                      0x00007000
+#define PKTSP_PORT1                     0x00001000
+#define TO_VLAN_MASK                    0x0003F000
+#define TO_VLAN_GROUP1                  0x00002000
+
+#define EOR_BIT                         BIT(31)
+
+/*define rx descriptor bit*/
+#define ERR_CODE                        (0xf << 26)
+#define RX_TCP_UDP_CHKSUM_BIT           BIT(23)
+#define RX_IP_CHKSUM_BIT                BIT(18)
+
+#define OWC_BIT                         BIT(31)
+#define TXOK_BIT                        BIT(26)
+#define LNKF_BIT                        BIT(25)
+#define BUR_BIT                         BIT(22)
+#define TWDE_BIT                        BIT(20)
+#define CC_MASK                         0x000f0000
+#define TBE_MASK                        0x00070000
+
+// Address table search
+#define MAC_ADDR_LOOKUP_IDLE            BIT(2)
+#define MAC_SEARCH_NEXT_ADDR            BIT(1)
+#define MAC_BEGIN_SEARCH_ADDR           BIT(0)
+
+// Address table search
+#define MAC_HASK_LOOKUP_ADDR_MASK       (0x3ff << 22)
+#define MAC_AT_TABLE_END                BIT(1)
+#define MAC_AT_DATA_READY               BIT(0)
+
+#define MAC_PHY_ADDR                    0x01	/* define by hardware */
+
+/*config descriptor*/
+#define TX_DESC_NUM                     16
+#define MAC_GUARD_DESC_NUM              2
+#define RX_QUEUE0_DESC_NUM              16
+#define RX_QUEUE1_DESC_NUM              16
+#define TX_DESC_QUEUE_NUM               1
+#define RX_DESC_QUEUE_NUM               2
+
+#define MAC_TX_BUFF_SIZE                1536
+#define MAC_RX_LEN_MAX                  2047
+
+#define DESC_ALIGN_BYTE                 32
+#define RX_OFFSET                       0
+#define TX_OFFSET                       0
+
+#define ETHERNET_MAC_ADDR_LEN           6
+
+struct mac_desc {
+	u32 cmd1;
+	u32 cmd2;
+	u32 addr1;
+	u32 addr2;
+};
+
+struct skb_info {
+	struct sk_buff *skb;
+	u32 mapping;
+	u32 len;
+};
+
+struct l2sw_common {
+	struct net_device *net_dev;
+	struct platform_device *pdev;
+	int dual_nic;
+	int sa_learning;
+
+	void *desc_base;
+	dma_addr_t desc_dma;
+	s32 desc_size;
+	struct clk *clk;
+	struct reset_control *rstc;
+	int irq;
+
+	struct mac_desc *rx_desc[RX_DESC_QUEUE_NUM];
+	struct skb_info *rx_skb_info[RX_DESC_QUEUE_NUM];
+	u32 rx_pos[RX_DESC_QUEUE_NUM];
+	u32 rx_desc_num[RX_DESC_QUEUE_NUM];
+	u32 rx_desc_buff_size;
+
+	struct mac_desc *tx_desc;
+	struct skb_info tx_temp_skb_info[TX_DESC_NUM];
+	u32 tx_done_pos;
+	u32 tx_pos;
+	u32 tx_desc_full;
+

From: Denis Kirjanov <[email protected]>
Sent: Wednesday, November 3, 2021 8:05 PM
To: Wells Lu <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Wells Lu 呂芳騰 <[email protected]>
Subject: Re: [PATCH 2/2] net: ethernet: Add driver for Sunplus SP7021

11/3/21 2:02 PM, Wells Lu пишет:

Quoted 13 lines

Add Ethernet driver for Sunplus SP7021.

Signed-off-by Wells Lu <[email protected]>

---
  MAINTAINERS                                  |   1 +
  drivers/net/ethernet/Kconfig                 |   1 +
  drivers/net/ethernet/Makefile                |   1 +
  drivers/net/ethernet/sunplus/Kconfig         |  20 +
  drivers/net/ethernet/sunplus/Makefile        |   6 +
  drivers/net/ethernet/sunplus/l2sw_define.h   | 221 ++++++++
  drivers/net/ethernet/sunplus/l2sw_desc.c     | 233 ++++++++
  drivers/net/ethernet/sunplus/l2sw_desc.h     |  21 +
  drivers/net/ethernet/sunplus/l2sw_driver.c   | 779

+++++++++++++++++++++++++++

Quoted 160 lines

  drivers/net/ethernet/sunplus/l2sw_driver.h   |  23 +
  drivers/net/ethernet/sunplus/l2sw_hal.c      | 422 +++++++++++++++
  drivers/net/ethernet/sunplus/l2sw_hal.h      |  47 ++
  drivers/net/ethernet/sunplus/l2sw_int.c      | 326 +++++++++++
  drivers/net/ethernet/sunplus/l2sw_int.h      |  16 +
  drivers/net/ethernet/sunplus/l2sw_mac.c      |  68 +++
  drivers/net/ethernet/sunplus/l2sw_mac.h      |  24 +
  drivers/net/ethernet/sunplus/l2sw_mdio.c     | 118 ++++
  drivers/net/ethernet/sunplus/l2sw_mdio.h     |  19 +
  drivers/net/ethernet/sunplus/l2sw_register.h |  99 ++++
  19 files changed, 2445 insertions(+)

create mode 100644 drivers/net/ethernet/sunplus/Kconfig create mode 100644 drivers/net/ethernet/sunplus/Makefile create mode 100644 drivers/net/ethernet/sunplus/l2sw_define.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_desc.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_desc.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_driver.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_driver.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_hal.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_hal.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_int.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_int.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_mac.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_mac.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_mdio.c create mode 100644 drivers/net/ethernet/sunplus/l2sw_mdio.h create mode 100644 drivers/net/ethernet/sunplus/l2sw_register.h

MAINTAINERS +1 -0

@@ -18006,6 +18006,7 @@ L:	[email protected]
  S:	Maintained
  W:	https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
  F:	Documentation/devicetree/bindings/net/sunplus,sp7021-l2sw.yaml
+F:	drivers/net/ethernet/sunplus/
 
  SUPERH
  M:	Yoshinori Sato <[email protected]>

(quoted diff) +0 -0

b/drivers/net/ethernet/Kconfig index 412ae3e..0084852 100644

drivers/net/ethernet/Kconfig +1 -0

@@ -176,6 +176,7 @@ source "drivers/net/ethernet/smsc/Kconfig"
  source "drivers/net/ethernet/socionext/Kconfig"
  source "drivers/net/ethernet/stmicro/Kconfig"
  source "drivers/net/ethernet/sun/Kconfig"
+source "drivers/net/ethernet/sunplus/Kconfig"
  source "drivers/net/ethernet/synopsys/Kconfig"
  source "drivers/net/ethernet/tehuti/Kconfig"
  source "drivers/net/ethernet/ti/Kconfig"

(quoted diff) +0 -0

b/drivers/net/ethernet/Makefile index aaa5078..e4ce162 100644

drivers/net/ethernet/Makefile +1 -0

@@ -87,6 +87,7 @@ obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
  obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
  obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
  obj-$(CONFIG_NET_VENDOR_SUN) += sun/
+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sunplus/
  obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
  obj-$(CONFIG_NET_VENDOR_TI) += ti/
  obj-$(CONFIG_NET_VENDOR_TOSHIBA) += toshiba/ diff --git

a/drivers/net/ethernet/sunplus/Kconfig
b/drivers/net/ethernet/sunplus/Kconfig
new file mode 100644
index 0000000..a9e3a4c

drivers/net/ethernet/sunplus/Kconfig new file +19 -0

@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Sunplus Ethernet device configuration #
+
+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
+	depends on ETHERNET && SOC_SP7021
+	select PHYLIB
+	select PINCTRL_SPPCTL
+	select COMMON_CLK_SP7021
+	select RESET_SUNPLUS
+	select NVMEM_SUNPLUS_OCOTP
+	help
+	  If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
+	  devices, say Y.
+	  The network device supports dual 10M/100M Ethernet interfaces,
+	  or one 10/100M Ethernet interface with two LAN ports.
+	  To compile this driver as a module, choose M here.  The module
+	  will be called sp_l2sw.

drivers/net/ethernet/sunplus/Makefile new file +7 -0

new file mode 100644
index 0000000..b401cec
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the Sunplus network device drivers.
+#
+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sp_l2sw.o sp_l2sw-objs :=
+l2sw_driver.o l2sw_int.o l2sw_hal.o l2sw_desc.o l2sw_mac.o
+l2sw_mdio.o

drivers/net/ethernet/sunplus/l2sw_define.h new file +50 -0

new file mode 100644
index 0000000..c1049c5
@@ -0,0 +1,221 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+
+#ifndef __L2SW_DEFINE_H__
+#define __L2SW_DEFINE_H__
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/kdev_t.h>
+#include <linux/in.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/ip.h>
+#include <linux/tcp.h>
+#include <linux/skbuff.h>
+#include <linux/ethtool.h>
+#include <linux/platform_device.h>
+#include <linux/phy.h>
+#include <linux/mii.h>
+#include <linux/if_vlan.h>
+#include <linux/io.h>
+#include <linux/dma-mapping.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+
+#undef pr_fmt
+#define pr_fmt(fmt)     "[L2SW]" fmt
+
+//define MAC interrupt status bit
+#define MAC_INT_DAISY_MODE_CHG          BIT(31)
+#define MAC_INT_IP_CHKSUM_ERR           BIT(23)
+#define MAC_INT_WDOG_TIMER1_EXP         BIT(22)
+#define MAC_INT_WDOG_TIMER0_EXP         BIT(21)
+#define MAC_INT_INTRUDER_ALERT          BIT(20)
+#define MAC_INT_PORT_ST_CHG             BIT(19)
+#define MAC_INT_BC_STORM                BIT(18)
+#define MAC_INT_MUST_DROP_LAN           BIT(17)
+#define MAC_INT_GLOBAL_QUE_FULL         BIT(16)
+#define MAC_INT_TX_SOC_PAUSE_ON         BIT(15)
+#define MAC_INT_RX_SOC_QUE_FULL         BIT(14)
+#define MAC_INT_TX_LAN1_QUE_FULL        BIT(9)
+#define MAC_INT_TX_LAN0_QUE_FULL        BIT(8)

drivers/net/ethernet/sunplus/l2sw_define.h line 60

Quoted 10 lines

drivers/net/ethernet/sunplus/l2sw_define.h new file +10 -0

+#define MAC_INT_RX_L_DESCF              BIT(7)
+#define MAC_INT_RX_H_DESCF              BIT(6)
+#define MAC_INT_RX_DONE_L               BIT(5)
+#define MAC_INT_RX_DONE_H               BIT(4)
+#define MAC_INT_TX_DONE_L               BIT(3)
+#define MAC_INT_TX_DONE_H               BIT(2)
+#define MAC_INT_TX_DES_ERR              BIT(1)
+#define MAC_INT_RX_DES_ERR              BIT(0)
+
+#define MAC_INT_RX                      (MAC_INT_RX_DONE_H |

MAC_INT_RX_DONE_L | \

drivers/net/ethernet/sunplus/l2sw_define.h line 62

Quoted 2 lines

drivers/net/ethernet/sunplus/l2sw_define.h new file +2 -0

+					MAC_INT_RX_DES_ERR)
+#define MAC_INT_TX                      (MAC_INT_TX_DONE_L |

MAC_INT_TX_DONE_H | \

drivers/net/ethernet/sunplus/l2sw_define.h line 64

Quoted 2 lines

drivers/net/ethernet/sunplus/l2sw_define.h new file +2 -0

+					MAC_INT_TX_DES_ERR)
+#define MAC_INT_MASK_DEF

(MAC_INT_DAISY_MODE_CHG | MAC_INT_IP_CHKSUM_ERR | \

drivers/net/ethernet/sunplus/l2sw_define.h line 65

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_define.h new file +1 -0

+					MAC_INT_WDOG_TIMER1_EXP |

MAC_INT_WDOG_TIMER0_EXP | \

drivers/net/ethernet/sunplus/l2sw_define.h line 66

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_define.h new file +1 -0

+					MAC_INT_INTRUDER_ALERT | MAC_INT_BC_STORM

| \

drivers/net/ethernet/sunplus/l2sw_define.h line 67

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_define.h new file +1 -0

+					MAC_INT_MUST_DROP_LAN |

MAC_INT_GLOBAL_QUE_FULL | \

drivers/net/ethernet/sunplus/l2sw_define.h line 68

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_define.h new file +1 -0

+					MAC_INT_TX_SOC_PAUSE_ON |

MAC_INT_RX_SOC_QUE_FULL | \

drivers/net/ethernet/sunplus/l2sw_define.h line 69

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_define.h new file +1 -0

+					MAC_INT_TX_LAN1_QUE_FULL |

MAC_INT_TX_LAN0_QUE_FULL | \

Quoted 50 lines

drivers/net/ethernet/sunplus/l2sw_define.h new file +50 -0

+					MAC_INT_RX_L_DESCF | MAC_INT_RX_H_DESCF)
+
+/*define port ability*/
+#define PORT_ABILITY_LINK_ST_P1         BIT(25)
+#define PORT_ABILITY_LINK_ST_P0         BIT(24)
+
+/*define PHY command bit*/
+#define PHY_WT_DATA_MASK                0xffff0000
+#define PHY_RD_CMD                      0x00004000
+#define PHY_WT_CMD                      0x00002000
+#define PHY_REG_MASK                    0x00001f00
+#define PHY_ADR_MASK                    0x0000001f
+
+/*define PHY status bit*/
+#define PHY_RD_DATA_MASK                0xffff0000
+#define PHY_RD_RDY                      BIT(1)
+#define PHY_WT_DONE                     BIT(0)
+
+/*define other register bit*/
+#define RX_MAX_LEN_MASK                 0x00011000
+#define ROUTE_MODE_MASK                 0x00000060
+#define POK_INT_THS_MASK                0x000E0000
+#define VLAN_TH_MASK                    0x00000007
+
+/*define tx descriptor bit*/
+#define OWN_BIT                         BIT(31)
+#define FS_BIT                          BIT(25)
+#define LS_BIT                          BIT(24)
+#define LEN_MASK                        0x000007FF
+#define PKTSP_MASK                      0x00007000
+#define PKTSP_PORT1                     0x00001000
+#define TO_VLAN_MASK                    0x0003F000
+#define TO_VLAN_GROUP1                  0x00002000
+
+#define EOR_BIT                         BIT(31)
+
+/*define rx descriptor bit*/
+#define ERR_CODE                        (0xf << 26)
+#define RX_TCP_UDP_CHKSUM_BIT           BIT(23)
+#define RX_IP_CHKSUM_BIT                BIT(18)
+
+#define OWC_BIT                         BIT(31)
+#define TXOK_BIT                        BIT(26)
+#define LNKF_BIT                        BIT(25)
+#define BUR_BIT                         BIT(22)
+#define TWDE_BIT                        BIT(20)
+#define CC_MASK                         0x000f0000
+#define TBE_MASK                        0x00070000
+
+// Address table search

drivers/net/ethernet/sunplus/l2sw_define.h

Quoted 10 lines

drivers/net/ethernet/sunplus/l2sw_define.h new file +10 -0

+#define MAC_ADDR_LOOKUP_IDLE            BIT(2)
+#define MAC_SEARCH_NEXT_ADDR            BIT(1)
+#define MAC_BEGIN_SEARCH_ADDR           BIT(0)
+
+// Address table search
+#define MAC_HASK_LOOKUP_ADDR_MASK       (0x3ff << 22)
+#define MAC_AT_TABLE_END                BIT(1)
+#define MAC_AT_DATA_READY               BIT(0)
+
+#define MAC_PHY_ADDR                    0x01	/* define by

hardware */

Quoted 60 lines

drivers/net/ethernet/sunplus/l2sw_define.h new file +60 -0

+
+/*config descriptor*/
+#define TX_DESC_NUM                     16
+#define MAC_GUARD_DESC_NUM              2
+#define RX_QUEUE0_DESC_NUM              16
+#define RX_QUEUE1_DESC_NUM              16
+#define TX_DESC_QUEUE_NUM               1
+#define RX_DESC_QUEUE_NUM               2
+
+#define MAC_TX_BUFF_SIZE                1536
+#define MAC_RX_LEN_MAX                  2047
+
+#define DESC_ALIGN_BYTE                 32
+#define RX_OFFSET                       0
+#define TX_OFFSET                       0
+
+#define ETHERNET_MAC_ADDR_LEN           6
+
+struct mac_desc {
+	u32 cmd1;
+	u32 cmd2;
+	u32 addr1;
+	u32 addr2;
+};
+
+struct skb_info {
+	struct sk_buff *skb;
+	u32 mapping;
+	u32 len;
+};
+
+struct l2sw_common {
+	struct net_device *net_dev;
+	struct platform_device *pdev;
+	int dual_nic;
+	int sa_learning;
+
+	void *desc_base;
+	dma_addr_t desc_dma;
+	s32 desc_size;
+	struct clk *clk;
+	struct reset_control *rstc;
+	int irq;
+
+	struct mac_desc *rx_desc[RX_DESC_QUEUE_NUM];
+	struct skb_info *rx_skb_info[RX_DESC_QUEUE_NUM];
+	u32 rx_pos[RX_DESC_QUEUE_NUM];
+	u32 rx_desc_num[RX_DESC_QUEUE_NUM];
+	u32 rx_desc_buff_size;
+
+	struct mac_desc *tx_desc;
+	struct skb_info tx_temp_skb_info[TX_DESC_NUM];
+	u32 tx_done_pos;
+	u32 tx_pos;
+	u32 tx_desc_full;
+
+	struct mii_bus *mii_bus;
+	struct phy_device *phy_dev;
+
+	struct napi_struct rx_napi;

drivers/net/ethernet/sunplus/l2sw_define.h line 195

Quoted 6 lines

drivers/net/ethernet/sunplus/l2sw_define.h new file +6 -0

+	struct napi_struct tx_napi;
+
+	spinlock_t rx_lock;      // spinlock for accessing rx buffer
+	spinlock_t tx_lock;      // spinlock for accessing tx buffer
+	spinlock_t ioctl_lock;   // spinlock for ioctl operations
+	struct mutex store_mode; // mutex for dynamic mode change

run checkpatch.pl and fix the errors

drivers/net/ethernet/sunplus/l2sw_driver.c new file +0 -0

new file mode 100644
index 0000000..3dfd0dd
@@ -0,0 +1,779 @@

drivers/net/ethernet/sunplus/l2sw_driver.c new file +5 -0

+static int l2sw_probe(struct platform_device *pdev)
+{
+	struct l2sw_common *comm;
+	struct resource *r_mem;
+	struct net_device *net_dev, *net_dev2;

drivers/net/ethernet/sunplus/l2sw_driver.c new file +21 -0

+	// Get memory resoruce 0 from dts.
+	r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r_mem) {
+		pr_debug(" res->name = \"%s\", r_mem->start = %pa\n", r_mem->name, &r_mem->start);
+		if (l2sw_reg_base_set(devm_ioremap(&pdev->dev, r_mem->start,
+						   (r_mem->end - r_mem->start + 1))) != 0) {
+			pr_err(" ioremap failed!\n");
+			ret = -ENOMEM;
+			goto out_free_comm;
+		}
+	} else {
+		pr_err(" No MEM resource 0 found!\n");
+		ret = -ENXIO;
+		goto out_free_comm;
+	}
+
+	// Get memory resoruce 1 from dts.
+	r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (r_mem) {
+		pr_debug(" res->name = \"%s\", r_mem->start = %pa\n", r_mem->name, &r_mem->start);
+		if (moon5_reg_base_set(devm_ioremap(&pdev->dev, r_mem->start,

From: Philipp Zabel <[email protected]>
Sent: Wednesday, November 3, 2021 8:10 PM
To: Wells Lu <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Wells Lu 呂芳騰 <[email protected]>
Subject: Re: [PATCH 2/2] net: ethernet: Add driver for Sunplus SP7021

On Wed, 2021-11-03 at 19:02 +0800, Wells Lu wrote:
[...]

Quoted 7 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +0 -0

new file mode 100644
index 0000000..3dfd0dd
@@ -0,0 +1,779 @@

[...]

Quoted 9 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +9 -0

+static int l2sw_probe(struct platform_device *pdev) {
+	struct l2sw_common *comm;
+	struct resource *r_mem;
+	struct net_device *net_dev, *net_dev2;
+	struct l2sw_mac *mac, *mac2;
+	u32 mode;
+	int ret = 0;
+	int rc;
+

drivers/net/ethernet/sunplus/l2sw_driver.c line 512

Quoted 5 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +5 -0

+	if (platform_get_drvdata(pdev))
+		return -ENODEV;
+
+	// Allocate memory for l2sw 'common' area.
+	comm = kmalloc(sizeof(*comm), GFP_KERNEL);

I'd use devm_kzalloc() here for initialization and to simplify the cleanup path.

drivers/net/ethernet/sunplus/l2sw_driver.c line 515

Quoted 3 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +3 -0

+	if (!comm)
+		return -ENOMEM;
+	pr_debug(" comm = %p\n", comm);

What is this useful for?

drivers/net/ethernet/sunplus/l2sw_driver.c line 516

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_driver.c new file +1 -0

+	memset(comm, '\0', sizeof(struct l2sw_common));

Not needed with kzalloc, See above.

[...]

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 4 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +4 -0

+	// Get memory resoruce 0 from dts.
+	r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (r_mem) {
+		pr_debug(" res->name = \"%s\", r_mem->start = %pa\n",

r_mem->name, &r_mem->start);

Quoted 6 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +6 -0

+		if (l2sw_reg_base_set(devm_ioremap(&pdev->dev, r_mem->start,
+						   (r_mem->end - r_mem->start + 1))) != 0) {
+			pr_err(" ioremap failed!\n");
+			ret = -ENOMEM;
+			goto out_free_comm;
+		}

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 10 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +10 -0

+	} else {
+		pr_err(" No MEM resource 0 found!\n");
+		ret = -ENXIO;
+		goto out_free_comm;
+	}
+
+	// Get memory resoruce 1 from dts.
+	r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (r_mem) {
+		pr_debug(" res->name = \"%s\", r_mem->start = %pa\n",

r_mem->name, &r_mem->start);

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_driver.c new file +1 -0

+		if (moon5_reg_base_set(devm_ioremap(&pdev->dev,

r_mem->start,

drivers/net/ethernet/sunplus/l2sw_driver.c line 554

Quoted 10 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +10 -0

+						    (r_mem->end - r_mem->start + 1))) != 0) {
+			pr_err(" ioremap failed!\n");
+			ret = -ENOMEM;
+			goto out_free_comm;
+		}
+	} else {
+		pr_err(" No MEM resource 1 found!\n");
+		ret = -ENXIO;
+		goto out_free_comm;
+	}

Using devm_ioremap_resource() would simplify both a lot.

[...]

drivers/net/ethernet/sunplus/l2sw_driver.c line 586

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_driver.c new file +1 -0

+	comm->rstc = devm_reset_control_get(&pdev->dev, NULL);

Please use devm_reset_control_get_exclusive().

Quoted 5 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +5 -0

+	if (IS_ERR(comm->rstc)) {
+		dev_err(&pdev->dev, "Failed to retrieve reset controller!\n");
+		ret = PTR_ERR(comm->rstc);
+		goto out_free_comm;
+	}

drivers/net/ethernet/sunplus/l2sw_driver.c line 597

Quoted 6 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +6 -0

+
+	// Enable clock.
+	clk_prepare_enable(comm->clk);
+	udelay(1);
+
+	ret = reset_control_assert(comm->rstc);

No need to assign to ret if you ignore it anyway.

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 4 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +4 -0

+	udelay(1);
+	ret = reset_control_deassert(comm->rstc);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to deassert reset line (err = %d)!\n",

ret);

drivers/net/ethernet/sunplus/l2sw_driver.c line 605

Quoted 4 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +4 -0

+		ret = -ENODEV;
+		goto out_free_comm;
+	}
+	udelay(1);

regards
Philipp

drivers/net/ethernet/sunplus/Kconfig new file +10 -0

new file mode 100644
index 0000000..a9e3a4c
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Sunplus Ethernet device configuration
+#
+
+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
+	depends on ETHERNET && SOC_SP7021
+	select PHYLIB
+	select PINCTRL_SPPCTL

Hi--

Quoted 16 lines · Wells Lu

drivers/net/ethernet/sunplus/Kconfig new file +9 -0

new file mode 100644
index 0000000..a9e3a4c
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Sunplus Ethernet device configuration #
+
+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
+	depends on ETHERNET && SOC_SP7021
+	select PHYLIB
+	select PINCTRL_SPPCTL

drivers/net/ethernet/sunplus/Kconfig line 19

Quoted 10 lines

drivers/net/ethernet/sunplus/Kconfig new file +10 -0

+	select COMMON_CLK_SP7021
+	select RESET_SUNPLUS
+	select NVMEM_SUNPLUS_OCOTP
+	help
+	  If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
+	  devices, say Y.
+	  The network device supports dual 10M/100M Ethernet interfaces,
+	  or one 10/100M Ethernet interface with two LAN ports.
+	  To compile this driver as a module, choose M here.  The module
+	  will be called sp_l2sw.

Please use NET_VENDOR_SUNPLUS in the same way that other NET_VENDOR_wyxz kconfig symbols are used. It should just enable or disable any specific device drivers under it.

--
~Randy

config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS

---help---
If you have a network (Ethernet) card belonging to this class, say Y here.

Note that the answer to this question doesn't directly affect the kernel: saying N will just cause the configurator to skip all the questions about Sunplus cards. If you say Y, you will be asked for your specific card in the following questions.

if NET_VENDOR_SUNPLUS

config SP7021_EMAC

	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
	depends on ETHERNET && SOC_SP7021

Quoted 5 lines

config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS

Does it actually depend on ARCH_SUNPLUS? What do you make use of?

Ideally, you want it to also build with COMPILE_TEST, so that the driver gets build by 0-day and all the other build bots.

Quoted 13 lines

---help---
If you have a network (Ethernet) card belonging to this class, say Y here.

Note that the answer to this question doesn't directly affect the kernel: saying N will just cause the configurator to skip all the questions about Sunplus cards. If you say Y, you will be asked for your specific card in the following questions.

if NET_VENDOR_SUNPLUS

Quoted 3 lines

config SP7021_EMAC

	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
	depends on ETHERNET && SOC_SP7021

Does it actually depend on SOC_SP7021 to build?

Andrew

Hi,

Thanks a lot for review.

Quoted 6 lines

Quoted 5 lines

config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS

Quoted 1 line

Does it actually depend on ARCH_SUNPLUS? What do you make use of?

ARCH_SUNPLUS will be defined for Sunplus family series SoC. Ethernet devices of Sunplus are designed and used for Sunplus SoC. So far, only two SoC of Sunplus have the network device. I'd like to show up the selection only for Sunplus SoC.

Quoted 2 lines

Ideally, you want it to also build with COMPILE_TEST, so that the driver gets build by 0-day and all the other build bots.

I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?

depends on ARCH_SUNPLUS | COMPILE_TEST

Yes, the device is now only for Sunplus SP7021 SoC. Devices in each SoC may have a bit difference because of adding new function or improving something.

Quoted 10 lines · Wells Lu 呂芳騰

Hi,

Thanks a lot for review.

Quoted 6 lines

Quoted 5 lines

config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS

Quoted 6 lines

Quoted 1 line

Does it actually depend on ARCH_SUNPLUS? What do you make use of?

ARCH_SUNPLUS will be defined for Sunplus family series SoC. Ethernet devices of Sunplus are designed and used for Sunplus SoC. So far, only two SoC of Sunplus have the network device. I'd like to show up the selection only for Sunplus SoC.

So it does not actually depend on ARCH_SUNPLUS. There are a few cases where drivers have needed to call into arch specific code, which stops them building for any other arch.

Quoted 3 lines

Quoted 2 lines

Ideally, you want it to also build with COMPILE_TEST, so that the driver gets build by 0-day and all the other build bots.

Quoted 4 lines

I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?

depends on ARCH_SUNPLUS | COMPILE_TEST

Yes.

Quoted 4 lines

Yes, the device is now only for Sunplus SP7021 SoC. Devices in each SoC may have a bit difference because of adding new function or improving something.

If it will compile with COMPILE_TEST on x86, mips, etc, you should allow it to compile with COMPILE_TEST. You get better compile testing that way.

Andrew

Quoted 25 lines · Wells Lu 呂芳騰

Quoted 10 lines

Hi,

Thanks a lot for review.

Quoted 6 lines

Quoted 5 lines

config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS

Quoted 6 lines

Quoted 1 line

Does it actually depend on ARCH_SUNPLUS? What do you make use of?

ARCH_SUNPLUS will be defined for Sunplus family series SoC. Ethernet devices of Sunplus are designed and used for Sunplus SoC. So far, only two SoC of Sunplus have the network device. I'd like to show up the selection only for Sunplus SoC.

So it does not actually depend on ARCH_SUNPLUS. There are a few cases where drivers have needed to call into arch specific code, which stops them building for any other arch.

Quoted 3 lines

Quoted 2 lines

Ideally, you want it to also build with COMPILE_TEST, so that the driver gets build by 0-day and all the other build bots.

Quoted 6 lines

Quoted 4 lines

I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?

depends on ARCH_SUNPLUS | COMPILE_TEST

Yes.

Yes, but use "||" instead of one "|".

Quoted 9 lines

Quoted 4 lines

Yes, the device is now only for Sunplus SP7021 SoC. Devices in each SoC may have a bit difference because of adding new function or improving something.

If it will compile with COMPILE_TEST on x86, mips, etc, you should allow it to compile with COMPILE_TEST. You get better compile testing that way.

Quoted 2 lines

Andrew

--
~Randy

Quoted 10 lines

Hi,

Thanks a lot for review.

Quoted 6 lines

Quoted 5 lines

config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS

Quoted 6 lines

Quoted 1 line

Does it actually depend on ARCH_SUNPLUS? What do you make use of?

ARCH_SUNPLUS will be defined for Sunplus family series SoC. Ethernet devices of Sunplus are designed and used for Sunplus SoC. So far, only two SoC of Sunplus have the network device. I'd like to show up the selection only for Sunplus SoC.

So it does not actually depend on ARCH_SUNPLUS. There are a few cases where drivers have needed to call into arch specific code, which stops them building for any other arch.

Quoted 7 lines

Quoted 2 lines

Ideally, you want it to also build with COMPILE_TEST, so that the driver gets build by 0-day and all the other build bots.

I am not sure if this is mandatory or not.
Should I add COMPILE_TEST as below?

depends on ARCH_SUNPLUS | COMPILE_TEST

Yes.

Quoted 3 lines

Yes, the device is now only for Sunplus SP7021 SoC. Devices in each SoC may have a bit difference because of adding new function or improving something.

If it will compile with COMPILE_TEST on x86, mips, etc, you should allow it to compile with COMPILE_TEST. You get better compile testing that way.

Andrew

No, we only develop arm-based SoC, never for x86 or mips. We never compile the driver for x86 or mips machine.

Quoted 2 lines

No, we only develop arm-based SoC, never for x86 or mips. We never compile the driver for x86 or mips machine.

You don't, but the Linux community does build for those architectures. Most people do tree wide refactoring work using x86. Tree wide cleanups using x86, etc. Any changes like that could touch your driver. The harder is it to build, the less build testing it will get, and tree wide changes which break it are less likely to get noticed. So you really do want it to compile cleanly for all architectures. If it does not, it normally actually means you are doing something wrong, something you need to fix anyway. So please do build it for x86 and make sure it builds cleanly.

Andrew

Quoted 36 lines

Hi--

Quoted 16 lines · Wells Lu

drivers/net/ethernet/sunplus/Kconfig new file +9 -0

new file mode 100644
index 0000000..a9e3a4c
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Sunplus Ethernet device configuration #
+
+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"
+	depends on ETHERNET && SOC_SP7021
+	select PHYLIB
+	select PINCTRL_SPPCTL

drivers/net/ethernet/sunplus/Kconfig line 19

Quoted 10 lines

drivers/net/ethernet/sunplus/Kconfig new file +10 -0

+	select COMMON_CLK_SP7021
+	select RESET_SUNPLUS
+	select NVMEM_SUNPLUS_OCOTP
+	help
+	  If you have Sunplus dual 10M/100M Ethernet (with L2 switch)
+	  devices, say Y.
+	  The network device supports dual 10M/100M Ethernet interfaces,
+	  or one 10/100M Ethernet interface with two LAN ports.
+	  To compile this driver as a module, choose M here.  The module
+	  will be called sp_l2sw.

Please use NET_VENDOR_SUNPLUS in the same way that other NET_VENDOR_wyxz kconfig symbols are used. It should just enable or disable any specific device drivers under it.

Quoted 2 lines

--
~Randy

I looked up Kconfig file of other vendors, but not sure what I should do. Do I need to modify Kconfig file in the form as shown below?

# SPDX-License-Identifier: GPL-2.0
#
# Sunplus device configuration
#

config NET_VENDOR_SUNPLUS
bool "Sunplus devices"
default y
depends on ARCH_SUNPLUS
---help---
If you have a network (Ethernet) card belonging to this class, say Y here.

Note that the answer to this question doesn't directly affect the kernel: saying N will just cause the configurator to skip all the questions about Sunplus cards. If you say Y, you will be asked for your specific card in the following questions.

if NET_VENDOR_SUNPLUS

config SP7021_EMAC
tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices" depends on ETHERNET && SOC_SP7021
select PHYLIB
select PINCTRL_SPPCTL
select COMMON_CLK_SP7021
select RESET_SUNPLUS
select NVMEM_SUNPLUS_OCOTP
help
If you have Sunplus dual 10M/100M Ethernet (with L2 switch) devices, say Y.
The network device supports dual 10M/100M Ethernet interfaces, or one 10/100M Ethernet interface with two LAN ports. To compile this driver as a module, choose M here. The module will be called sp_l2sw.

endif # NET_VENDOR_SUNPLUS

Best regards,
Wells

+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sp_l2sw.o

+struct l2sw_common {

drivers/net/ethernet/sunplus/l2sw_driver.c new file +7 -0

+			return comm->phy2_addr;
+		else
+			return comm->phy1_addr;
+
+	case SIOCGMIIREG:
+		spin_lock_irqsave(&comm->ioctl_lock, flags);
+		data->val_out = mdio_read(data->phy_id, data->reg_num);

drivers/net/ethernet/sunplus/l2sw_hal.c new file +4 -0

+
+	ndev->mtu = new_mtu;
+
+	return 0;

+++ b/drivers/net/ethernet/sunplus/l2sw_mdio.c

drivers/net/ethernet/sunplus/l2sw_mdio.c new file +49 -0

@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+
+#include "l2sw_mdio.h"
+
+static int mii_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+	return mdio_read(phy_id, regnum);
+}
+
+static int mii_write(struct mii_bus *bus, int phy_id, int regnum, u16 val)
+{
+	return mdio_write(phy_id, regnum, val);
+}
+
+u32 mdio_init(struct platform_device *pdev, struct net_device *net_dev)
+{
+	struct l2sw_mac *mac = netdev_priv(net_dev);
+	struct mii_bus *mii_bus;
+	struct device_node *mdio_node;
+	u32 ret;
+
+	mii_bus = mdiobus_alloc();
+	if (!mii_bus) {
+		pr_err(" Failed to allocate mdio_bus memory!\n");
+		return -ENOMEM;
+	}
+
+	mii_bus->name = "sunplus_mii_bus";
+	mii_bus->parent = &pdev->dev;
+	mii_bus->priv = mac;
+	mii_bus->read = mii_read;
+	mii_bus->write = mii_write;
+	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev));
+
+	mdio_node = of_get_parent(mac->comm->phy1_node);
+	ret = of_mdiobus_register(mii_bus, mdio_node);
+	if (ret) {
+		pr_err(" Failed to register mii bus (ret = %d)!\n", ret);
+		mdiobus_free(mii_bus);
+		return ret;
+	}
+
+	mac->comm->mii_bus = mii_bus;
+	return ret;
+}
+

drivers/net/ethernet/sunplus/Kconfig line 7

Quoted 2 lines

drivers/net/ethernet/sunplus/Kconfig new file +2 -0

+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"

The "with L2 Switch" is causing lots of warning bells to ring for me.

I don't see any references to switchdev or DSA in this driver. How is the switch managed? There have been a few examples in the past of similar two port switches being first supported in Dual MAC mode. Later trying to actually use the switch in the Linux was always ran into problems, and basically needed a new driver. So i want to make sure you don't have this problem.

In the Linux world, Ethernet switches default to having there ports/interfaces separated. This effectively gives you your dual MAC mode by default. You then create a Linux bridge, and add the ports/interfaces to the bridge. switchdev is used to offload the bridge, telling the hardware to enable the L2 switch between the ports.

So you don't need the mode parameter in DT. switchdev tells you this. Switchdev gives user space access to the address table etc.

Quoted 2 lines

+obj-$(CONFIG_NET_VENDOR_SUNPLUS) += sp_l2sw.o

...

Quoted 1 line

+struct l2sw_common {

Please change your prefix. l2sw is a common prefix, there are other silicon vendors using l2sw. I would suggest sp_l2sw or spl2sw.

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 9 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +9 -0

+static int ethernet_do_ioctl(struct net_device *net_dev, struct ifreq
+*ifr, int cmd) {
+	struct l2sw_mac *mac = netdev_priv(net_dev);
+	struct l2sw_common *comm = mac->comm;
+	struct mii_ioctl_data *data = if_mii(ifr);
+	unsigned long flags;
+
+	pr_debug(" if = %s, cmd = %04x\n", ifr->ifr_ifrn.ifrn_name, cmd);
+	pr_debug(" phy_id = %d, reg_num = %d, val_in = %04x\n",

data->phy_id,

drivers/net/ethernet/sunplus/l2sw_driver.c line 171

Quoted 1 line

drivers/net/ethernet/sunplus/l2sw_driver.c new file +1 -0

+		 data->reg_num, data->val_in);

You should not be using any of the pr_ functions. You have a net_dev, so netdev_dbg().

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 5 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +5 -0

+
+	// Check parameters' range.
+	if ((cmd == SIOCGMIIREG) || (cmd == SIOCSMIIREG)) {
+		if (data->reg_num > 31) {
+			pr_err(" reg_num (= %d) excesses range!\n",

(int)data->reg_num);

Don't spam the kernel log for things like this.

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 8 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +8 -0

+			return -EINVAL;
+		}
+	}
+
+	switch (cmd) {
+	case SIOCGMIIPHY:
+		if (comm->dual_nic && (strcmp(ifr->ifr_ifrn.ifrn_name, "eth1") ==
+0))

You cannot rely on the name, systemd has probably renamed it. If you have using phylib correctly, net_dev->phydev is what you want.

Quoted 9 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +7 -0

+			return comm->phy2_addr;
+		else
+			return comm->phy1_addr;
+
+	case SIOCGMIIREG:
+		spin_lock_irqsave(&comm->ioctl_lock, flags);
+		data->val_out = mdio_read(data->phy_id, data->reg_num);

drivers/net/ethernet/sunplus/l2sw_driver.c line 200

Quoted 10 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +10 -0

+		spin_unlock_irqrestore(&comm->ioctl_lock, flags);
+		pr_debug(" val_out = %04x\n", data->val_out);
+		break;
+
+	case SIOCSMIIREG:
+		spin_lock_irqsave(&comm->ioctl_lock, flags);
+		mdio_write(data->phy_id, data->reg_num, data->val_in);
+		spin_unlock_irqrestore(&comm->ioctl_lock, flags);
+		break;
+

You should be using phylink_mii_ioctl() or phy_mii_ioctl().

You locking is also suspect.

drivers/net/ethernet/sunplus/l2sw_driver.c line 214

Quoted 7 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +7 -0

+static int ethernet_change_mtu(struct net_device *ndev, int new_mtu)
+{
+	if (netif_running(ndev))
+		return -EBUSY;
+
+	if (new_mtu < 68 || new_mtu > ETH_DATA_LEN)
+		return -EINVAL;

The core will do this for you, if you set the values in the ndev correct at probe time.

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 13 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +11 -0

+
+	ndev->mtu = new_mtu;
+
+	return 0;
+}
+
 
 
+static int mdio_access(u8 op_cd, u8 dev_reg_addr, u8 phy_addr, u32
+wdata) {
+	u32 value, time = 0;
+
+	HWREG_W(phy_cntl_reg0, (wdata << 16) | (op_cd << 13) |

(dev_reg_addr << 8) | phy_addr);

Review

Quoted 1 line

(quoted diff) +1 -0

+	wmb();			// make sure settings are effective.

That suggests you are using the wrong macros to access the registers.

drivers/net/ethernet/sunplus/l2sw_hal.c line 337

Quoted 8 lines

drivers/net/ethernet/sunplus/l2sw_hal.c new file +8 -0

+	do {
+		if (++time > MDIO_RW_TIMEOUT_RETRY_NUMBERS) {
+			pr_err(" mdio failed to operate!\n");
+			time = 0;
+		}
+
+		value = HWREG_R(phy_cntl_reg1);
+	} while ((value & 0x3) == 0);

include/linux/iopoll.h.

drivers/net/ethernet/sunplus/l2sw_hal.c line 340

Quoted 2 lines

drivers/net/ethernet/sunplus/l2sw_hal.c new file +2 -0

+	if (time == 0)
+		return -1;

-ETIMDEOUT. One of the advantages of iopoll.h is that reusing code avoids issues like this.

Review

Quoted 2 lines

(quoted diff) +2 -0

+u32 mdio_read(u32 phy_id, u16 regnum) {
+	int ret;

Please check for C45 and return -EOPNOTSUPP.

Quoted 5 lines

drivers/net/ethernet/sunplus/l2sw_hal.c new file +5 -0

+
+	ret = mdio_access(MDIO_READ_CMD, regnum, phy_id, 0);
+	if (ret < 0)
+		return -EIO;
+

drivers/net/ethernet/sunplus/l2sw_hal.c line 359

Quoted 6 lines

drivers/net/ethernet/sunplus/l2sw_hal.c new file +6 -0

+	return ret;
+}
+
+u32 mdio_write(u32 phy_id, u32 regnum, u16 val) {
+	int ret;
+

Please check for C45 and return -EOPNOTSUPP.

drivers/net/ethernet/sunplus/l2sw_hal.c line 367

Quoted 2 lines

drivers/net/ethernet/sunplus/l2sw_hal.c new file +2 -0

+
+inline void tx_trigger(void)

No inline functions in C code. Let the compiler decide.

drivers/net/ethernet/sunplus/l2sw_hal.c

Quoted 6 lines

drivers/net/ethernet/sunplus/l2sw_hal.c new file +6 -0

+int phy_cfg(struct l2sw_mac *mac)
+{
+	// Bug workaround:
+	// Flow-control of phy should be enabled. L2SW IP flow-control will refer
+	// to the bit to decide to enable or disable flow-control.
+	mdio_write(mac->comm->phy1_addr, 4,

mdio_read(mac->comm->phy1_addr, 4) | (1 << 10));

Review

Quoted 1 line

(quoted diff) +1 -0

+	mdio_write(mac->comm->phy2_addr, 4,

mdio_read(mac->comm->phy2_addr,

Quoted 1 line

+4) | (1 << 10));

This should be in the PHY driver. The MAC driver should never need to touch PHY registers.

Quoted 54 lines

+++ b/drivers/net/ethernet/sunplus/l2sw_mdio.c

drivers/net/ethernet/sunplus/l2sw_mdio.c new file +52 -0

@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ */
+
+#include "l2sw_mdio.h"
+
+static int mii_read(struct mii_bus *bus, int phy_id, int regnum) {
+	return mdio_read(phy_id, regnum);
+}
+
+static int mii_write(struct mii_bus *bus, int phy_id, int regnum, u16
+val) {
+	return mdio_write(phy_id, regnum, val); }
+
+u32 mdio_init(struct platform_device *pdev, struct net_device
+*net_dev) {
+	struct l2sw_mac *mac = netdev_priv(net_dev);
+	struct mii_bus *mii_bus;
+	struct device_node *mdio_node;
+	u32 ret;
+
+	mii_bus = mdiobus_alloc();
+	if (!mii_bus) {
+		pr_err(" Failed to allocate mdio_bus memory!\n");
+		return -ENOMEM;
+	}
+
+	mii_bus->name = "sunplus_mii_bus";
+	mii_bus->parent = &pdev->dev;
+	mii_bus->priv = mac;
+	mii_bus->read = mii_read;
+	mii_bus->write = mii_write;
+	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "%s-mii",
+dev_name(&pdev->dev));
+
+	mdio_node = of_get_parent(mac->comm->phy1_node);
+	ret = of_mdiobus_register(mii_bus, mdio_node);
+	if (ret) {
+		pr_err(" Failed to register mii bus (ret = %d)!\n", ret);
+		mdiobus_free(mii_bus);
+		return ret;
+	}
+
+	mac->comm->mii_bus = mii_bus;
+	return ret;
+}
+
+void mdio_remove(struct net_device *net_dev) {
+	struct l2sw_mac *mac = netdev_priv(net_dev);
+
+	if (mac->comm->mii_bus) {

drivers/net/ethernet/sunplus/l2sw_mdio.c line 59

Quoted 5 lines

drivers/net/ethernet/sunplus/l2sw_mdio.c new file +5 -0

+		mdiobus_unregister(mac->comm->mii_bus);
+		mdiobus_free(mac->comm->mii_bus);
+		mac->comm->mii_bus = NULL;
+	}
+}

You MDIO code is pretty scattered around. Please bring it all together in one file.

Review line 58

Quoted 1 line

(quoted diff) +1 -0

+static void mii_linkchange(struct net_device *netdev) { }

Nothing to do? Seems very odd. Don't you need to tell the MAC it should do 10Mbps or 100Mbps? What about pause?

drivers/net/ethernet/sunplus/l2sw_mdio.c

Quoted 7 lines

drivers/net/ethernet/sunplus/l2sw_mdio.c new file +7 -0

+
+int mac_phy_probe(struct net_device *netdev) {
+	struct l2sw_mac *mac = netdev_priv(netdev);
+	struct phy_device *phydev;
+	int i;
+
+	phydev = of_phy_connect(mac->net_dev, mac->comm->phy1_node,

mii_linkchange,

Review line 66

Quoted 1 line

(quoted diff) +1 -0

+				0, PHY_INTERFACE_MODE_RGMII_ID);

You should not hard code PHY_INTERFACE_MODE_RGMII_ID. Use the DT property "phy-mode"

drivers/net/ethernet/sunplus/l2sw_mdio.c line 75

Quoted 3 lines

drivers/net/ethernet/sunplus/l2sw_mdio.c new file +3 -0

+	if (!phydev) {
+		pr_err(" \"%s\" has no phy found\n", netdev->name);
+		return -1;

-ENODEV;

Never use -1, pick an error code.

drivers/net/ethernet/sunplus/l2sw_mdio.c

Quoted 5 lines

drivers/net/ethernet/sunplus/l2sw_mdio.c new file +4 -0

+	}
+
+	if (mac->comm->phy2_node) {
+		of_phy_connect(mac->net_dev, mac->comm->phy2_node,

mii_linkchange,

drivers/net/ethernet/sunplus/l2sw_mdio.c

Quoted 4 lines

drivers/net/ethernet/sunplus/l2sw_mdio.c new file +4 -0

+			       0, PHY_INTERFACE_MODE_RGMII_ID);
+	}
+
+	linkmode_clear_bit(ETHTOOL_LINK_MODE_Pause_BIT,

phydev->supported);

Review line 79

Quoted 2 lines

(quoted diff) +2 -0

+	linkmode_clear_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
+phydev->supported);

So the MAC does not support pause? I'm then confused about phy_cfg().

Andrew

Quoted 12 lines

drivers/net/ethernet/sunplus/Kconfig line 7

Quoted 2 lines

drivers/net/ethernet/sunplus/Kconfig new file +2 -0

+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"

The "with L2 Switch" is causing lots of warning bells to ring for me.

I don't see any references to switchdev or DSA in this driver. How is the switch managed? There have been a few examples in the past of similar two port switches being first supported in Dual MAC mode. Later trying to actually use the switch in the Linux was always ran into problems, and basically needed a new driver. So i want to make sure you don't have this problem.

Quoted 8 lines

In the Linux world, Ethernet switches default to having there ports/interfaces separated. This effectively gives you your dual MAC mode by default. You then create a Linux bridge, and add the ports/interfaces to the bridge. switchdev is used to offload the bridge, telling the hardware to enable the L2 switch between the ports.

So you don't need the mode parameter in DT. switchdev tells you this. Switchdev gives user space access to the address table etc.

The L2 switch of Ethernet of SP7021 is not used to forward packets between two network interfaces.

Sunplus Dual Ethernet devices consists of one CPU port, two LAN ports, and a L2 switch. L2 switch is a circuitry which receives packets from CPU or LAN ports and then forwards them other ports. Rules of forwarding packets are set by driver.

Ethernet driver of SP7021 of Sunplus supports 3 operation modes:
- Dual NIC mode
- An NIC with two LAN ports mode (daisy-chain mode)
- An NIC with two LAN ports mode 2

Dual NIC mode
Ethernet driver creates two net-device interfaces (eg: eth0 and eth1). Each has its dedicated LAN port. For example, LAN port 0 is for net-device interface eth0. LAN port 1 is for net-device interface eth1. Packets from LAN port 0 will be always forwarded to eth0 and vice versa by L2 switch. Similarly, packets from LAN port 1 will be always forwarded to eth1 and vice versa by L2 switch. Packets will never be forwarded between two LAN ports, or between eth0 and LAN port 1, or between eth1 and LAN port 0. The two network devices work independently.

An NIC with two LAN ports mode (daisy-chain mode) Ethernet driver creates one net-device interface (eg: eth0), but the net-device interface has two LAN ports. In this mode, a packet from one LAN port will be either forwarded to net-device interface (eht0) if its destination address matches MAC address of net-device interface (eth0), or forwarded to other LAN port. A packet from net-device interface (eth0) will be forwarded to a LAN port if its destination address is learnt by L2 switch, or forwarded to both LAN ports if its destination has not been learnt yet.

An NIC with two LAN ports mode 2
This mode is similar to “An NIC with two LAN ports mode”. The difference is that a packet from net-device interface (eth0) will be always forwarded to both LAN ports. Learning function of L2 switch is turned off in this mode. This means L2 switch will never learn the source address of a packet. So, it always forward packets to both LAN ports. This mode works like you have 2-port Ethernet hub.

Quoted 4 lines

Quoted 1 line

+struct l2sw_common {

Please change your prefix. l2sw is a common prefix, there are other silicon vendors using l2sw. I would suggest sp_l2sw or spl2sw.

Ok, I'll modify two struct names in next patch as shown below:
l2sw_common --> sp_common
l2sw_mac --> sp_mac

Should I also modify prefix of file name?

Quoted 11 lines

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 8 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +8 -0

+			return -EINVAL;
+		}
+	}
+
+	switch (cmd) {
+	case SIOCGMIIPHY:
+		if (comm->dual_nic && (strcmp(ifr->ifr_ifrn.ifrn_name, "eth1") ==
+0))

You cannot rely on the name, systemd has probably renamed it. If you have using phylib correctly, net_dev->phydev is what you want.

Ok, I'll use name of the second net device to do compare, instead of using fixed string "eth1", in next patch.

Quoted 11 lines

drivers/net/ethernet/sunplus/l2sw_hal.c

Quoted 6 lines

drivers/net/ethernet/sunplus/l2sw_hal.c new file +6 -0

+int phy_cfg(struct l2sw_mac *mac)
+{
+	// Bug workaround:
+	// Flow-control of phy should be enabled. L2SW IP flow-control will refer
+	// to the bit to decide to enable or disable flow-control.
+	mdio_write(mac->comm->phy1_addr, 4,

mdio_read(mac->comm->phy1_addr, 4) | (1 << 10));

Review

Quoted 1 line

(quoted diff) +1 -0

+	mdio_write(mac->comm->phy2_addr, 4,

mdio_read(mac->comm->phy2_addr,

Quoted 1 line

+4) | (1 << 10));

Quoted 2 lines

This should be in the PHY driver. The MAC driver should never need to touch PHY registers.

Sunplus Ethernet MAC integrates MDIO controller. So Ethernet driver has MDIO- and PHY-related code. To work-around a circuitry bug, we need to enable bit 10 of register 4 of PHY.
Where should we place the code?

Quoted 4 lines

Quoted 1 line

+static void mii_linkchange(struct net_device *netdev) { }

Nothing to do? Seems very odd. Don't you need to tell the MAC it should do 10Mbps or 100Mbps? What about pause?

No, hardware does it automatically.
Sunplus MAC integrates MDIO controller.
It reads PHY status and set MAC automatically.

Quoted 1 line

So the MAC does not support pause? I'm then confused about phy_cfg().

Yes, MAC supports pause. MAC (hardware) takes care of pause automatically.

Should I remove the two lines?

Quoted 55 lines

Quoted 12 lines

drivers/net/ethernet/sunplus/Kconfig line 7

Quoted 2 lines

drivers/net/ethernet/sunplus/Kconfig new file +2 -0

+config NET_VENDOR_SUNPLUS
+	tristate "Sunplus Dual 10M/100M Ethernet (with L2 switch) devices"

The "with L2 Switch" is causing lots of warning bells to ring for me.

I don't see any references to switchdev or DSA in this driver. How is the switch managed? There have been a few examples in the past of similar two port switches being first supported in Dual MAC mode. Later trying to actually use the switch in the Linux was always ran into problems, and basically needed a new driver. So i want to make sure you don't have this problem.

Quoted 8 lines

In the Linux world, Ethernet switches default to having there ports/interfaces separated. This effectively gives you your dual MAC mode by default. You then create a Linux bridge, and add the ports/interfaces to the bridge. switchdev is used to offload the bridge, telling the hardware to enable the L2 switch between the ports.

So you don't need the mode parameter in DT. switchdev tells you this. Switchdev gives user space access to the address table etc.

The L2 switch of Ethernet of SP7021 is not used to forward packets between two network interfaces.

Sunplus Dual Ethernet devices consists of one CPU port, two LAN ports, and a L2 switch. L2 switch is a circuitry which receives packets from CPU or LAN ports and then forwards them other ports. Rules of forwarding packets are set by driver.

Ethernet driver of SP7021 of Sunplus supports 3 operation modes:
- Dual NIC mode
- An NIC with two LAN ports mode (daisy-chain mode)
- An NIC with two LAN ports mode 2

Dual NIC mode
Ethernet driver creates two net-device interfaces (eg: eth0 and eth1). Each has its dedicated LAN port. For example, LAN port 0 is for net-device interface eth0. LAN port 1 is for net-device interface eth1. Packets from LAN port 0 will be always forwarded to eth0 and vice versa by L2 switch. Similarly, packets from LAN port 1 will be always forwarded to eth1 and vice versa by L2 switch. Packets will never be forwarded between two LAN ports, or between eth0 and LAN port 1, or between eth1 and LAN port 0. The two network devices work independently.

An NIC with two LAN ports mode (daisy-chain mode) Ethernet driver creates one net-device interface (eg: eth0), but the net-device interface has two LAN ports. In this mode, a packet from one LAN port will be either forwarded to net-device interface (eht0) if its destination address matches MAC address of net-device interface (eth0), or forwarded to other LAN port. A packet from net-device interface (eth0) will be forwarded to a LAN port if its destination address is learnt by L2 switch, or forwarded to both LAN ports if its destination has not been learnt yet.

Quoted 7 lines

An NIC with two LAN ports mode 2
This mode is similar to “An NIC with two LAN ports mode”. The difference is that a packet from net-device interface (eth0) will be always forwarded to both LAN ports. Learning function of L2 switch is turned off in this mode. This means L2 switch will never learn the source address of a packet. So, it always forward packets to both LAN ports. This mode works like you have 2-port Ethernet hub.

So here you describe how the hardware can be used. Dual is two interfaces. Daisy-chain is what you get by taking those two interfaces and adding them to a bridge. The bridge then forwards frames between the interfaces and the CPU as needed, based on learning. And your third mode is the bridge always performs flooding.

A linux driver must follow the linux networking model. You cannot make up your own model. In the linux world, you model the external ports. The hardware always has two external ports, so you need to always have two netdev interfaces. To bridge packets between those two interfaces, you create a bridge and you add the interfaces to the bridge. That is the model you need to follow. switchdev gives you the API calls you need to implement this.

Quoted 5 lines

Quoted 4 lines

Quoted 1 line

+struct l2sw_common {

Please change your prefix. l2sw is a common prefix, there are other silicon vendors using l2sw. I would suggest sp_l2sw or spl2sw.

Quoted 5 lines

Ok, I'll modify two struct names in next patch as shown below:
l2sw_common --> sp_common
l2sw_mac --> sp_mac

Should I also modify prefix of file name?

You need to modify the prefix everywhere you use it. Function names, variable names, all symbols. Search and replace throughout the whole code.

Quoted 13 lines

Quoted 12 lines

drivers/net/ethernet/sunplus/l2sw_driver.c

Quoted 9 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +9 -0

+			return -EINVAL;
+		}
+	}
+
+	switch (cmd) {
+	case SIOCGMIIPHY:
+		if (comm->dual_nic && (strcmp(ifr->ifr_ifrn.ifrn_name, "eth1")
+==
+0))

You cannot rely on the name, systemd has probably renamed it. If you have using phylib correctly, net_dev->phydev is what you want.

Quoted 2 lines

Ok, I'll use name of the second net device to do compare, instead of using fixed string "eth1", in next patch.

No. There are always two interfaces. You always have two netdev structures. Each netdev structure has a phydev. So use netdev->phydev.

This is another advantage of the Linux model. In your daisy chain mode, how do i control the two PHYs? How do i see one is up and one is down? How do i configure one to 10Half and the other 100Full?

Quoted 3 lines

Quoted 3 lines

Review

Quoted 3 lines

(quoted diff) +3 -0

+int phy_cfg(struct l2sw_mac *mac) {
+	// Bug workaround:
+	// Flow-control of phy should be enabled. L2SW IP flow-control will

refer

Quoted 7 lines

Quoted 7 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+	// to the bit to decide to enable or disable flow-control.
+	mdio_write(mac->comm->phy1_addr, 4,

mdio_read(mac->comm->phy1_addr, 4) | (1 << 10));

Review

Quoted 1 line

(quoted diff) +1 -0

+	mdio_write(mac->comm->phy2_addr, 4,

mdio_read(mac->comm->phy2_addr,

Quoted 1 line

+4) | (1 << 10));

Quoted 8 lines

Quoted 2 lines

This should be in the PHY driver. The MAC driver should never need to touch PHY registers.

Sunplus Ethernet MAC integrates MDIO controller.
So Ethernet driver has MDIO- and PHY-related code.
To work-around a circuitry bug, we need to enable bit 10 of register 4 of PHY.
Where should we place the code?

The silicon is integrated, but it is still a collection of standard blocks. Linux models those blocks independently. There is a subsystem for the MAC, a subsystem for the MDIO bus master and a subsystem for the PHY. You register a driver with each of these subsystems. PHY drivers live in drivers/net/phy. Put a PHY driver in there, which includes this workaround.

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

+static void mii_linkchange(struct net_device *netdev) { }

Nothing to do? Seems very odd. Don't you need to tell the MAC it should do 10Mbps or 100Mbps? What about pause?

No, hardware does it automatically.
Sunplus MAC integrates MDIO controller.
It reads PHY status and set MAC automatically.

The PHY is external? So you have no idea what PHY that is? It could be a Marvell PHY, a microchip PHY, an Atheros PHY. Often PHYs have pages. In order to read the temperature sensor you change the page, read a register, and then hopefully change the page back again. If the PHY supports Fibre as well as copper, it can put the fibre registers in a second page. The PHY driver knows about this, it will flip the pages as needed. The phylib core has a mutex, so that only one operation happens at a time. So a page flip does not happen unexpectedly.

Your MAC hardware does not take this mutex. It has no idea what page is selected when it reads registers. Instead of getting the basic mode register, it could get the LED control register...

The MAC should never directly access the PHY. Please disable this hardware, and use the mii_linkchange callback to configure the MAC.

Quoted 6 lines

Quoted 1 line

So the MAC does not support pause? I'm then confused about phy_cfg().

Yes, MAC supports pause. MAC (hardware) takes care of pause automatically.

Should I remove the two lines?

Yes.

And you need to configure the MAC based on the results of the auto-neg.

Andrew

SP7021 Ethernet supports 3 operation modes:
- Dual Ethernet mode
In this mode, driver creates two net-device interfaces. Each connects to PHY. There are two LAN ports totally.
I am sorry that EMAC of SP7021 cannot support L2 switch functions of Linux switch-device model because it only has partial function of switch.

- One Ethernet mode
In this mode, driver creates one net-device interface. It connects to to a PHY (There is only one LAN port).
The LAN port is then connected to a 3-port Ethernet hub. The 3-port Ethernet hub is a hardware circuitry. All operations (packet forwarding) are done by hardware. No software intervention is needed. Actually, even just power-on, no software running, two LAN ports of SP7021 work well as 2-port hub.

Quoted 7 lines

SP7021 Ethernet supports 3 operation modes:
- Dual Ethernet mode
In this mode, driver creates two net-device interfaces. Each connects to PHY. There are two LAN ports totally.
I am sorry that EMAC of SP7021 cannot support L2 switch functions of Linux switch-device model because it only has partial function of switch.

This is fine.

Quoted 9 lines

- One Ethernet mode
In this mode, driver creates one net-device interface. It connects to to a PHY (There is only one LAN port).
The LAN port is then connected to a 3-port Ethernet hub. The 3-port Ethernet hub is a hardware circuitry. All operations (packet forwarding) are done by hardware. No software intervention is needed. Actually, even just power-on, no software running, two LAN ports of SP7021 work well as 2-port hub.

We need to dig into the details of this mode. I would initially say no, until we really do know it is impossible to do it correctly. Even if it is impossible to do it correctly, i'm still temped to reject this mode.

How does spanning tree work? Who sends and receives the BPDU?

Is there PTP support? How do you send and receive the PTP frames?

Is IGMP snooping supported?

All of these have one thing in common, you need to be able to egress frames out a specific port of the switch, and you need to know what port a received frames ingressed on. If you can do that, you can probably do proper support in Linux.

Is the datasheet available?

Andrew

The switch will not recognize type of packets, regardless BPDU, PTP or any other packets. If turning off source-address learning function, it works like an Ethernet plus a 2-port hub.

Quoted 3 lines

The switch will not recognize type of packets, regardless BPDU, PTP or any other packets. If turning off source-address learning function, it works like an Ethernet plus a 2-port hub.

So without STP, there is no way to stop an loop, and a broadcast storm taking down your network?

Looking at the TX descriptor, there are two bits:

[18]: force forward to port 0
[19]: force forward to port 1

When the switch is enabled, can these two bits be used?

In the RX descriptor there is:

pkt_sp:

          000: from port0
          001: from port1
          110: soc0 loopback
          101: soc1 loopback

Are these bits used when the switch is enabled?

0.31 port control 1 (port cntl1) blocking state seems to have what you need for STP.

Andrew

Quoted 6 lines

Quoted 3 lines

The switch will not recognize type of packets, regardless BPDU, PTP or any other packets. If turning off source-address learning function, it works like an Ethernet plus a 2-port hub.

So without STP, there is no way to stop an loop, and a broadcast storm taking down your network?

Do you mean connecting two PHY ports to the same LAN? We never connect two PHY ports to the same LAN (or hub). I never think of this loop problem. I thought only WAN has the loop problem.

How an Ethernet hub take care of this situation?

Is that reasonable to connect two ports of an Ethernet hub together?

Quoted 6 lines

Looking at the TX descriptor, there are two bits:

[18]: force forward to port 0
[19]: force forward to port 1

When the switch is enabled, can these two bits be used?

Yes, for example, when bit 19 of TX descriptor is enabled, a packet from CPU port is forwarded to LAN port 0 forcibly.

Quoted 8 lines

In the RX descriptor there is:

pkt_sp:

          000: from port0
          001: from port1
          110: soc0 loopback
          101: soc1 loopback

Quoted 1 line

Are these bits used when the switch is enabled?

Yes, E- MAC driver uses these bits to tell where a packet comes from. Note that soc1 port (CPU port) has been removed in this chip.

Sorry, I don't know what is a RMC packet?

Quoted 10 lines

Quoted 6 lines

Quoted 3 lines

The switch will not recognize type of packets, regardless BPDU, PTP or any other packets. If turning off source-address learning function, it works like an Ethernet plus a 2-port hub.

So without STP, there is no way to stop an loop, and a broadcast storm taking down your network?

Do you mean connecting two PHY ports to the same LAN? We never connect two PHY ports to the same LAN (or hub). I never think of this loop problem. I thought only WAN has the loop problem.

Any Ethernet network can have a loop. Often loops a deliberate because they give redundancy. STP will detect this loop, and somewhere in the network one of the switches will block traffic to break the loop. But if something in the network breaks, the port can be unblocked to allow traffic to flow, redundancy. Well behaved switches should always implement STP.

Quoted 1 line

How an Ethernet hub take care of this situation?

STP. Run tcpdump on your network. Depending on how your network is configured, you might see BPDU from your building switches.

Quoted 1 line

Is that reasonable to connect two ports of an Ethernet hub together?

It is not just together. You cannot guarantee any Ethernet network is a tree. You could connect the two ports to two different hubs, but those hubs are connected together, and so you get a loop.

Quoted 19 lines

Quoted 6 lines

Looking at the TX descriptor, there are two bits:

[18]: force forward to port 0
[19]: force forward to port 1

When the switch is enabled, can these two bits be used?

Yes, for example, when bit 19 of TX descriptor is enabled, a packet from CPU port is forwarded to LAN port 0 forcibly.

Quoted 8 lines

In the RX descriptor there is:

pkt_sp:

          000: from port0
          001: from port1
          110: soc0 loopback
          101: soc1 loopback

Quoted 4 lines

Quoted 1 line

Are these bits used when the switch is enabled?

Yes, E- MAC driver uses these bits to tell where a packet comes from. Note that soc1 port (CPU port) has been removed in this chip.

Right. So you can have two netdev when in L2 switch mode.

You need to think about the Linux model some more. In linux, networking hardware is there to accelerate what the Linux stack can do in software. Take for example a simple SoC will have two Ethernet interfaces. You can perform software bridging on those two interfaces:

ip link add name br0 type bridge
ip link set dev br0 up
ip link set dev eth0 master br0
ip link set dev eth1 master br0

The software bridge will decided which interface to send a packet out. The software will perform learning etc.

You can use your dual MAC setup exactly like this. But you can also go further. You can use the hardware to accelerate switching packets between eth0 and eth1. But also Linux can still send packets out specific ports using these bits. The software bridge and the hardware bridge work together. This is the correct way to do this in Linux.

Quoted 1 line

Sorry, I don't know what is a RMC packet?

Sorry, i have no idea.

Andrew

I don't know how to implement STP in L2 switch like SP7021.

If this is acceptable, I'd like to have Ethernet of SP7021 have two operation modes:
- Dual NIC mode
- Single NIC with 2-port frame-flooding hub mode

RMII pins of PHY ports of SP7021 are multiplexable. I'd like to switch RMII pins of the second PHY for other use if single NIC mode is used. In fact, some SP7021 boards have dual Ethernet and some have only one Ethernet. We really need the two operation modes.

After looking up some data, I find RMC means reserved multi-cast.
RMC packets means packets with DA = 0x0180c2000000, 0x0180c2000002 ~ 0x0180c200000f, except the PAUSE packet (DA = 0x0180c2000001)

Quoted 1 line

I don't know how to implement STP in L2 switch like SP7021.

That is the nice thing about using Linux. It already knows how to implement STP. The bridge will do it for you. You just need to add the callbacks in the driver which are needed. Please take a look at other switchdev drivers.

Quoted 5 lines

If this is acceptable, I'd like to have Ethernet of SP7021 have two operation
modes:
- Dual NIC mode
- Single NIC with 2-port frame-flooding hub mode

No, sorry. Do it correctly, or do not do it. Please start with a clean driver doing Dual NIC mode. You can add L2 support later, once you have done the research to understand switchdev, etc.

Quoted 4 lines

RMII pins of PHY ports of SP7021 are multiplexable. I'd like to switch RMII pins of the second PHY for other use if single NIC mode is used. In fact, some SP7021 boards have dual Ethernet and some have only one Ethernet. We really need the two operation modes.

Only using a subset of ports in a switch is common. The common binding for DSA switches is described in:

Documentation/devicetree/bindings/net/dsa/dsa.yaml and for example Documentation/devicetree/bindings/net/dsa/hirschmann,hellcreek.yaml is a memory mapped switch. Notice the reg numbers:

           ethernet-ports {
                #address-cells = <1>;
                #size-cells = <0>;

                port@0 {
                    reg = <0>;
                    label = "cpu";
                    ethernet = <&gmac0>;
                };

                port@2 {
                    reg = <2>;
                    label = "lan0";
                    phy-handle = <&phy1>;
                };

reg = <1> is missing in this example. Port 1 of the switch is not used. You can do the same with a 2 port switch, when you don't want to make use of a port. Just don't list it in DT.

Quoted 3 lines

After looking up some data, I find RMC means reserved multi-cast. RMC packets means packets with DA = 0x0180c2000000, 0x0180c2000002 ~ 0x0180c200000f, except the PAUSE packet (DA = 0x0180c2000001)

Ah, good. BPDUs use 01:80:C2:00:00:00. So they will be passed when the port is in blocking mode. PTP uses 01:80:C2:00:00:0E. So the hardware designers appear to of designed a proper L2 switch with everything you need for a managed switch. What is missing is software. The more i learn about this hardware, the more i've convinced you need to write proper Linux support for it, not your mode hacks.

Andrew

drivers/net/ethernet/sunplus/l2sw_driver.c new file +8 -0

+static int l2sw_remove(struct platform_device *pdev)
+{
+	struct net_device *net_dev;
+	struct net_device *net_dev2;
+	struct l2sw_mac *mac;
+
+	net_dev = platform_get_drvdata(pdev);
+	if (!net_dev)

drivers/net/ethernet/sunplus/l2sw_driver.c new file +10 -0

+	sysfs_remove_group(&pdev->dev.kobj, &l2sw_attribute_group);
+
+	mac->comm->enable = 0;
+	soc0_stop(mac);
+
+	napi_disable(&mac->comm->rx_napi);
+	netif_napi_del(&mac->comm->rx_napi);
+	napi_disable(&mac->comm->tx_napi);
+	netif_napi_del(&mac->comm->tx_napi);
+

Hi, Wells!

On 11/3/21 14:02, Wells Lu wrote:

[code snip]

drivers/net/ethernet/sunplus/l2sw_driver.c line 426

Quoted 8 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +8 -0

+		if (comm->dual_nic) {
+			struct net_device *net_dev2 = mac->next_netdev;
+
+			if (!netif_running(net_dev2)) {
+				mac_hw_stop(mac);
+
+				mac2 = netdev_priv(net_dev2);
+

(*)

drivers/net/ethernet/sunplus/l2sw_driver.c line 437

Quoted 11 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +11 -0

+				// unregister and free net device.
+				unregister_netdev(net_dev2);
+				free_netdev(net_dev2);
+				mac->next_netdev = NULL;
+				pr_info(" Unregistered and freed net device \"eth1\"!\n");
+
+				comm->dual_nic = 0;
+				mac_switch_mode(mac);
+				rx_mode_set(net_dev);
+				mac_hw_addr_del(mac2);
+

mac2 is net_dev2 private data (*), so it will become freed after free_netdev() call.

FWIW the latest smatch should warn about this type of bugs.

drivers/net/ethernet/sunplus/l2sw_driver.c line 441

Quoted 4 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +4 -0

+				// If eth0 is up, turn on lan 0 and 1 when
+				// switching to daisy-chain mode.
+				if (comm->enable & 0x1)
+					comm->enable = 0x3;

[code snip]

Quoted 12 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +12 -0

+static int l2sw_remove(struct platform_device *pdev) {
+	struct net_device *net_dev;
+	struct net_device *net_dev2;
+	struct l2sw_mac *mac;
+
+	net_dev = platform_get_drvdata(pdev);
+	if (!net_dev)
+		return 0;
+	mac = netdev_priv(net_dev);
+
+	// Unregister and free 2nd net device.
+	net_dev2 = mac->next_netdev;

drivers/net/ethernet/sunplus/l2sw_driver.c line 454

Quoted 5 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +5 -0

+	if (net_dev2) {
+		unregister_netdev(net_dev2);
+		free_netdev(net_dev2);
+	}
+

Is it save here to free mac->next_netdev before unregistering "parent" netdev? I haven't checked the whole code, just asking :)

Quoted 14 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +14 -0

+	sysfs_remove_group(&pdev->dev.kobj, &l2sw_attribute_group);
+
+	mac->comm->enable = 0;
+	soc0_stop(mac);
+
+	napi_disable(&mac->comm->rx_napi);
+	netif_napi_del(&mac->comm->rx_napi);
+	napi_disable(&mac->comm->tx_napi);
+	netif_napi_del(&mac->comm->tx_napi);
+
+	mdio_remove(net_dev);
+
+	// Unregister and free 1st net device.
+	unregister_netdev(net_dev);

drivers/net/ethernet/sunplus/l2sw_driver.c line 754

Quoted 6 lines

drivers/net/ethernet/sunplus/l2sw_driver.c new file +6 -0

+	free_netdev(net_dev);
+
+	clk_disable(mac->comm->clk);
+
+	// Free 'common' area.
+	kfree(mac->comm);

Same here with mac.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	return 0;
+}

I haven't read the whole thread, i am sorry if these questions were already discussed.

With regards,
Pavel Skripkin

Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and etc.) into a single chip. It is designed for industrial control applications.

Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html

Wells Lu (2):
devicetree: bindings: net: Add bindings doc for Sunplus SP7021.
net: ethernet: Add driver for Sunplus SP7021

  .../bindings/net/sunplus,sp7021-l2sw.yaml          | 123 ++++
  MAINTAINERS                                        |   8 +
  drivers/net/ethernet/Kconfig                       |   1 +
  drivers/net/ethernet/Makefile                      |   1 +
  drivers/net/ethernet/sunplus/Kconfig               |  20 +
  drivers/net/ethernet/sunplus/Makefile              |   6 +
  drivers/net/ethernet/sunplus/l2sw_define.h         | 221 ++++++
  drivers/net/ethernet/sunplus/l2sw_desc.c           | 233 ++++++
  drivers/net/ethernet/sunplus/l2sw_desc.h           |  21 +
  drivers/net/ethernet/sunplus/l2sw_driver.c         | 779 +++++++++++++++++++++
  drivers/net/ethernet/sunplus/l2sw_driver.h         |  23 +
  drivers/net/ethernet/sunplus/l2sw_hal.c            | 422 +++++++++++
  drivers/net/ethernet/sunplus/l2sw_hal.h            |  47 ++
  drivers/net/ethernet/sunplus/l2sw_int.c            | 326 +++++++++
  drivers/net/ethernet/sunplus/l2sw_int.h            |  16 +
  drivers/net/ethernet/sunplus/l2sw_mac.c            |  68 ++
  drivers/net/ethernet/sunplus/l2sw_mac.h            |  24 +
  drivers/net/ethernet/sunplus/l2sw_mdio.c           | 118 ++++
  drivers/net/ethernet/sunplus/l2sw_mdio.h           |  19 +
  drivers/net/ethernet/sunplus/l2sw_register.h       |  99 +++
  20 files changed, 2575 insertions(+)

create mode 100644 Documentation/devicetree/bindings/net/sunplus,sp7021-l2sw.yaml create mode 100644 drivers/net/ethernet/sunplus/Kconfig
create mode 100644 drivers/net/ethernet/sunplus/Makefile
create mode 100644 drivers/net/ethernet/sunplus/l2sw_define.h
create mode 100644 drivers/net/ethernet/sunplus/l2sw_desc.c
create mode 100644 drivers/net/ethernet/sunplus/l2sw_desc.h
create mode 100644 drivers/net/ethernet/sunplus/l2sw_driver.c
create mode 100644 drivers/net/ethernet/sunplus/l2sw_driver.h
create mode 100644 drivers/net/ethernet/sunplus/l2sw_hal.c
create mode 100644 drivers/net/ethernet/sunplus/l2sw_hal.h
create mode 100644 drivers/net/ethernet/sunplus/l2sw_int.c
create mode 100644 drivers/net/ethernet/sunplus/l2sw_int.h
create mode 100644 drivers/net/ethernet/sunplus/l2sw_mac.c
create mode 100644 drivers/net/ethernet/sunplus/l2sw_mac.h
create mode 100644 drivers/net/ethernet/sunplus/l2sw_mdio.c
create mode 100644 drivers/net/ethernet/sunplus/l2sw_mdio.h
create mode 100644 drivers/net/ethernet/sunplus/l2sw_register.h

Add bindings documentation for Sunplus SP7021.

Signed-off-by Wells Lu <[email protected]>

---
Changes in V2
- Added mdio and phy sub-nodes.

 .../bindings/net/sunplus,sp7021-emac.yaml          | 152 +++++++++++++++++++++
 MAINTAINERS                                        |   7 +
 2 files changed, 159 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml

Quoted 7 lines

Add bindings documentation for Sunplus SP7021.

Signed-off-by Wells Lu <[email protected]>

---
Changes in V2
- Added mdio and phy sub-nodes.

Quoted 6 lines

 .../bindings/net/sunplus,sp7021-emac.yaml          | 152 +++++++++++++++++++++
 MAINTAINERS                                        |   7 +
 2 files changed, 159 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml:94:12: [warning] wrong
indentation: expected 10 but found 11 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1553831

This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml new file +14 -0

+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    emac: emac@9c108000 {
+        compatible = "sunplus,sp7021-emac";
+        reg = <0x9c108000 0x400>, <0x9c000280 0x80>;
+        reg-names = "emac", "moon5";
+        interrupt-parent = <&intc>;
+        interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clkc 0xa7>;
+        resets = <&rstc 0x97>;
+        phy-handle1 = <&eth_phy0>;
+        phy-handle2 = <&eth_phy1>;
+        pinctrl-0 = <&emac_demo_board_v3_pins>;

Quoted 18 lines

Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml new file +18 -0

+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    emac: emac@9c108000 {
+        compatible = "sunplus,sp7021-emac";
+        reg = <0x9c108000 0x400>, <0x9c000280 0x80>;
+        reg-names = "emac", "moon5";
+        interrupt-parent = <&intc>;
+        interrupts = <66 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clkc 0xa7>;
+        resets = <&rstc 0x97>;
+        phy-handle1 = <&eth_phy0>;
+        phy-handle2 = <&eth_phy1>;
+        pinctrl-0 = <&emac_demo_board_v3_pins>;
+        pinctrl-names = "default";
+        nvmem-cells = <&mac_addr0>, <&mac_addr1>;
+        nvmem-cell-names = "mac_addr0", "mac_addr1";
+

Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml line 144

Quoted 6 lines

Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml new file +6 -0

+        mdio {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            eth_phy0: ethernet-phy@0 {
+                reg = <0>;
+                phy-mode = "rmii";

This is in the wrong place. It is a MAC property. You usually put it next to phy-handle.

Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml line 150

Quoted 6 lines

Documentation/devicetree/bindings/net/sunplus,sp7021-emac.yaml new file +6 -0

+            };
+            eth_phy1: ethernet-phy@1 {
+                reg = <1>;
+                phy-mode = "rmii";
+            };
+        };

I would suggest you structure this differently to make it clear it is a two port switch:

	ethernet-ports {
		#address-cells = <1>;
                #size-cells = <0>;

                port@0 {
                    reg = <0>;
		    phy-handle = <&eth_phy0>;
		    phy-mode = "rmii";
		}

		port@1 {
                    reg = <1>;
		    phy-handle = <&eth_phy1>;
		    phy-mode = "rmii";
		}
	}

	Andrew