[PATCH net-next 0/7] support more link mode for TXGBE

[PATCH net-next 0/7] support more link mode for TXGBE

[Jiawen Wu]

There are three new interface mode support for Wangxun 10Gb NICs:
1000BASE-X, SGMII and XAUI.

Specific configurations are added to XPCS. And external PHY attaching
is added for copper NICs. 

Jiawen Wu (7):
  net: pcs: xpcs: add specific vendor supoprt for Wangxun 10Gb NICs
  net: pcs: xpcs: support to switch mode for Wangxun NICs
  net: pcs: xpcs: add 1000BASE-X AN interrupt support
  net: pcs: xpcs: adapt Wangxun NICs for SGMII mode
  net: txgbe: support switching mode to 1000BASE-X and SGMII
  net: txgbe: support copper NIC with external PHY
  net: ngbe: move mdio access registers to wxlib

 MAINTAINERS                                   |   1 +
 drivers/net/ethernet/wangxun/Kconfig          |   1 +
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  28 +++
 drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c |  84 ++++----
 drivers/net/ethernet/wangxun/ngbe/ngbe_type.h |  19 --
 .../ethernet/wangxun/txgbe/txgbe_ethtool.c    |   9 +
 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c |  41 +++-
 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h |   2 +
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  83 ++++++--
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 191 +++++++++++++++++-
 drivers/net/pcs/Makefile                      |   2 +-
 drivers/net/pcs/pcs-xpcs-wx.c                 | 190 +++++++++++++++++
 drivers/net/pcs/pcs-xpcs.c                    |  85 +++++++-
 drivers/net/pcs/pcs-xpcs.h                    |  16 ++
 include/linux/pcs/pcs-xpcs.h                  |   3 +
 15 files changed, 662 insertions(+), 93 deletions(-)
 create mode 100644 drivers/net/pcs/pcs-xpcs-wx.c

-- 
2.27.0

[PATCH net-next 1/7] net: pcs: xpcs: add specific vendor supoprt for Wangxun 10Gb NICs

[Jiawen Wu]

Since Wangxun 10Gb NICs require some special configuration on the IP of
Synopsys Designware XPCS, the vendor identification of wangxun devices
is added by comparing the name of mii bus.

And interrupt mode is used in Wangxun devices, so make it to be the first
specific configuration.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/pcs/pcs-xpcs.c   | 11 ++++++++++-
 include/linux/pcs/pcs-xpcs.h |  2 ++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 44b037646865..79a34ffb7518 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -238,6 +238,14 @@ static int xpcs_write_vpcs(struct dw_xpcs *xpcs, int reg, u16 val)
 	return xpcs_write_vendor(xpcs, MDIO_MMD_PCS, reg, val);
 }
 
+static bool xpcs_dev_is_txgbe(struct dw_xpcs *xpcs)
+{
+	if (!strcmp(xpcs->mdiodev->bus->name, DW_MII_BUS_TXGBE))
+		return true;
+
+	return false;
+}
+
 static int xpcs_poll_reset(struct dw_xpcs *xpcs, int dev)
 {
 	/* Poll until the reset bit clears (50ms per retry == 0.6 sec) */
@@ -1289,7 +1297,8 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
 		if (compat->an_mode == DW_10GBASER)
 			return xpcs;
 
-		xpcs->pcs.poll = true;
+		if (!xpcs_dev_is_txgbe(xpcs))
+			xpcs->pcs.poll = true;
 
 		ret = xpcs_soft_reset(xpcs, compat);
 		if (ret)
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index ff99cf7a5d0d..fb60c7e28623 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -20,6 +20,8 @@
 #define DW_AN_C37_1000BASEX		4
 #define DW_10GBASER			5
 
+#define DW_MII_BUS_TXGBE	"txgbe_pcs_mdio_bus"
+
 struct xpcs_id;
 
 struct dw_xpcs {
-- 
2.27.0

Re: [PATCH net-next 1/7] net: pcs: xpcs: add specific vendor supoprt for Wangxun 10Gb NICs

[Andrew Lunn]

> +static bool xpcs_dev_is_txgbe(struct dw_xpcs *xpcs)
> +{
> +	if (!strcmp(xpcs->mdiodev->bus->name, DW_MII_BUS_TXGBE))
> +		return true;
> +
> +	return false;
> +}

> +#define DW_MII_BUS_TXGBE	"txgbe_pcs_mdio_bus"

I don't see much value in using a #define here. Just use the string
directly.

	Andrew

[PATCH net-next 2/7] net: pcs: xpcs: support to switch mode for Wangxun NICs

[Jiawen Wu]

According to chapter 6 of DesignWare Cores Ethernet PCS (version 3.20a)
and custom design manual, add a configuration flow for switching interface
mode.

If the interface changes, the following setting is required:
1. wait VR_XS_PCS_DIG_STS bit(4, 2) [PSEQ_STATE] = 100b (Power-Good)
2. write SR_XS_PCS_CTRL2 to select various PCS type
3. write SR_PMA_CTRL1 and/or SR_XS_PCS_CTRL1 for link speed
4. program PMA registers
5. write VR_XS_PCS_DIG_CTRL1 bit(15) [VR_RST] = 1b (Vendor-Specific
   Soft Reset)
6. wait for VR_XS_PCS_DIG_CTRL1 bit(15) [VR_RST] to get cleared

Only 10GBASE-R/SGMII/1000BASE-X modes are planned for the current Wangxun
devices. And there is a quirk for Wangxun devices to switch mode although
the interface in phylink state has not changed, since PCS will change to
default 10GBASE-R when the ethernet driver(txgbe) do LAN reset.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 MAINTAINERS                   |   1 +
 drivers/net/pcs/Makefile      |   2 +-
 drivers/net/pcs/pcs-xpcs-wx.c | 189 ++++++++++++++++++++++++++++++++++
 drivers/net/pcs/pcs-xpcs.c    |  12 ++-
 drivers/net/pcs/pcs-xpcs.h    |   7 ++
 include/linux/pcs/pcs-xpcs.h  |   1 +
 6 files changed, 207 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/pcs/pcs-xpcs-wx.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 990e3fce753c..feebec1f97ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22887,6 +22887,7 @@ S:	Maintained
 W:	https://www.net-swift.com
 F:	Documentation/networking/device_drivers/ethernet/wangxun/*
 F:	drivers/net/ethernet/wangxun/
+F:	drivers/net/pcs/pcs-xpcs-wx.c
 
 WATCHDOG DEVICE DRIVERS
 M:	Wim Van Sebroeck <wim@linux-watchdog.org>
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index ea662a7989b2..fb1694192ae6 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for Linux PCS drivers
 
-pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-nxp.o
+pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-nxp.o pcs-xpcs-wx.o
 
 obj-$(CONFIG_PCS_XPCS)		+= pcs_xpcs.o
 obj-$(CONFIG_PCS_LYNX)		+= pcs-lynx.o
diff --git a/drivers/net/pcs/pcs-xpcs-wx.c b/drivers/net/pcs/pcs-xpcs-wx.c
new file mode 100644
index 000000000000..6d1df34958bd
--- /dev/null
+++ b/drivers/net/pcs/pcs-xpcs-wx.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2015 - 2023 Beijing WangXun Technology Co., Ltd. */
+
+#include <linux/pcs/pcs-xpcs.h>
+#include <linux/mdio.h>
+#include "pcs-xpcs.h"
+
+/* VR_XS_PMA_MMD */
+#define TXGBE_PMA_MMD			0x8020
+#define TXGBE_TX_GENCTL1		0x11
+#define TXGBE_TX_GENCTL1_VBOOST_LVL	GENMASK(10, 8)
+#define TXGBE_TX_GENCTL1_VBOOST_EN0	BIT(4)
+#define TXGBE_TX_GEN_CTL2		0x12
+#define TXGBE_TX_RATE_CTL		0x14
+#define TXGBE_RX_GEN_CTL2		0x32
+#define TXGBE_RX_GEN_CTL3		0x33
+#define TXGBE_RX_GEN_CTL3_LOS_TRSHLD0	GENMASK(2, 0)
+#define TXGBE_RX_RATE_CTL		0x34
+#define TXGBE_RX_EQ_ATTN_CTL		0x37
+#define TXGBE_RX_EQ_ATTN_LVL0		GENMASK(2, 0)
+#define TXGBE_RX_EQ_CTL0		0x38
+#define TXGBE_RX_EQ_CTL4		0x3C
+#define TXGBE_RX_EQ_CTL4_CONT_ADAPT0	BIT(0)
+#define TXGBE_AFE_DFE_ENABLE		0x3D
+#define TXGBE_DFE_EN_0			BIT(4)
+#define TXGBE_AFE_EN_0			BIT(0)
+#define TXGBE_DFE_TAP_CTL0		0x3E
+#define TXGBE_MPLLA_CTL0		0x51
+#define TXGBE_MPLLA_CTL2		0x53
+#define TXGBE_MPLLA_CTL3		0x57
+#define TXGBE_MISC_CTL0			0x70
+#define TXGBE_VCO_CAL_LD0		0x72
+#define TXGBE_VCO_CAL_REF0		0x76
+
+static int txgbe_read_pma(struct dw_xpcs *xpcs, int reg)
+{
+	return xpcs_read(xpcs, MDIO_MMD_PMAPMD, TXGBE_PMA_MMD + reg);
+}
+
+static int txgbe_write_pma(struct dw_xpcs *xpcs, int reg, u16 val)
+{
+	return xpcs_write(xpcs, MDIO_MMD_PMAPMD, TXGBE_PMA_MMD + reg, val);
+}
+
+static void txgbe_pma_config_10gbaser(struct dw_xpcs *xpcs)
+{
+	int val;
+
+	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL0, 0x21);
+	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL3, 0);
+	val = txgbe_read_pma(xpcs, TXGBE_TX_GENCTL1);
+	val = u16_replace_bits(val, 0x5, TXGBE_TX_GENCTL1_VBOOST_LVL);
+	txgbe_write_pma(xpcs, TXGBE_TX_GENCTL1, val);
+	txgbe_write_pma(xpcs, TXGBE_MISC_CTL0, 0xCF00);
+	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_LD0, 0x549);
+	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_REF0, 0x29);
+	txgbe_write_pma(xpcs, TXGBE_TX_RATE_CTL, 0);
+	txgbe_write_pma(xpcs, TXGBE_RX_RATE_CTL, 0);
+	txgbe_write_pma(xpcs, TXGBE_TX_GEN_CTL2, 0x300);
+	txgbe_write_pma(xpcs, TXGBE_RX_GEN_CTL2, 0x300);
+	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL2, 0x600);
+
+	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL0, 0x45);
+	val = txgbe_read_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL);
+	val &= ~TXGBE_RX_EQ_ATTN_LVL0;
+	txgbe_write_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL, val);
+	txgbe_write_pma(xpcs, TXGBE_DFE_TAP_CTL0, 0xBE);
+	val = txgbe_read_pma(xpcs, TXGBE_AFE_DFE_ENABLE);
+	val &= ~(TXGBE_DFE_EN_0 | TXGBE_AFE_EN_0);
+	txgbe_write_pma(xpcs, TXGBE_AFE_DFE_ENABLE, val);
+	val = txgbe_read_pma(xpcs, TXGBE_RX_EQ_CTL4);
+	val &= ~TXGBE_RX_EQ_CTL4_CONT_ADAPT0;
+	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL4, val);
+}
+
+static void txgbe_pma_config_1g(struct dw_xpcs *xpcs)
+{
+	int val;
+
+	val = txgbe_read_pma(xpcs, TXGBE_TX_GENCTL1);
+	val = u16_replace_bits(val, 0x5, TXGBE_TX_GENCTL1_VBOOST_LVL);
+	val &= ~TXGBE_TX_GENCTL1_VBOOST_EN0;
+	txgbe_write_pma(xpcs, TXGBE_TX_GENCTL1, val);
+	txgbe_write_pma(xpcs, TXGBE_MISC_CTL0, 0xCF00);
+
+	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL0, 0x7706);
+	val = txgbe_read_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL);
+	val &= ~TXGBE_RX_EQ_ATTN_LVL0;
+	txgbe_write_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL, val);
+	txgbe_write_pma(xpcs, TXGBE_DFE_TAP_CTL0, 0);
+	val = txgbe_read_pma(xpcs, TXGBE_RX_GEN_CTL3);
+	val = u16_replace_bits(val, 0x4, TXGBE_RX_GEN_CTL3_LOS_TRSHLD0);
+	txgbe_write_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL, val);
+
+	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL0, 0x20);
+	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL3, 0x46);
+	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_LD0, 0x540);
+	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_REF0, 0x2A);
+	txgbe_write_pma(xpcs, TXGBE_AFE_DFE_ENABLE, 0);
+	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL4, 0x10);
+	txgbe_write_pma(xpcs, TXGBE_TX_RATE_CTL, 0x3);
+	txgbe_write_pma(xpcs, TXGBE_RX_RATE_CTL, 0x3);
+	txgbe_write_pma(xpcs, TXGBE_TX_GEN_CTL2, 0x100);
+	txgbe_write_pma(xpcs, TXGBE_RX_GEN_CTL2, 0x100);
+	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL2, 0x200);
+}
+
+static int txgbe_pcs_poll_power_up(struct dw_xpcs *xpcs)
+{
+	int val, ret;
+
+	/* Wait xpcs power-up good */
+	ret = read_poll_timeout(xpcs_read_vpcs, val,
+				(val & DW_PSEQ_ST) == DW_PSEQ_ST_GOOD,
+				10000, 1000000, false,
+				xpcs, DW_VR_XS_PCS_DIG_STS);
+	if (ret < 0)
+		pr_err("%s: xpcs power-up timeout\n", __func__);
+
+	return ret;
+}
+
+static int txgbe_pma_init_done(struct dw_xpcs *xpcs)
+{
+	int val, ret;
+
+	xpcs_write_vpcs(xpcs, DW_VR_XS_PCS_DIG_CTRL1, DW_VR_RST);
+
+	/* wait pma initialization done */
+	ret = read_poll_timeout(xpcs_read_vpcs, val, !(val & DW_VR_RST),
+				100000, 10000000, false,
+				xpcs, DW_VR_XS_PCS_DIG_CTRL1);
+	if (ret < 0)
+		pr_err("%s: xpcs pma initialization timeout\n", __func__);
+
+	return ret;
+}
+
+static bool txgbe_xpcs_mode_quirk(struct dw_xpcs *xpcs)
+{
+	int ret;
+
+	/* When txgbe do LAN reset, PCS will change to default 10GBASE-R mode */
+	ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_CTRL2);
+	ret &= MDIO_PCS_CTRL2_TYPE;
+	if (ret == MDIO_PCS_CTRL2_10GBR &&
+	    xpcs->interface != PHY_INTERFACE_MODE_10GBASER)
+		return true;
+
+	return false;
+}
+
+int txgbe_xpcs_switch_mode(struct dw_xpcs *xpcs, phy_interface_t interface)
+{
+	int val, ret;
+
+	switch (interface) {
+	case PHY_INTERFACE_MODE_10GBASER:
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_1000BASEX:
+		break;
+	default:
+		return 0;
+	}
+
+	if (xpcs->interface == interface && !txgbe_xpcs_mode_quirk(xpcs))
+		return 0;
+
+	xpcs->interface = interface;
+
+	ret = txgbe_pcs_poll_power_up(xpcs);
+	if (ret < 0)
+		return ret;
+
+	if (interface == PHY_INTERFACE_MODE_10GBASER) {
+		xpcs_write(xpcs, MDIO_MMD_PCS, MDIO_CTRL2, MDIO_PCS_CTRL2_10GBR);
+		val = xpcs_read(xpcs, MDIO_MMD_PMAPMD, MDIO_CTRL1);
+		val |= MDIO_CTRL1_SPEED10G;
+		xpcs_write(xpcs, MDIO_MMD_PMAPMD, MDIO_CTRL1, val);
+		txgbe_pma_config_10gbaser(xpcs);
+	} else {
+		xpcs_write(xpcs, MDIO_MMD_PCS, MDIO_CTRL2, MDIO_PCS_CTRL2_10GBX);
+		xpcs_write(xpcs, MDIO_MMD_PMAPMD, MDIO_CTRL1, 0);
+		xpcs_write(xpcs, MDIO_MMD_PCS, MDIO_CTRL1, 0);
+		txgbe_pma_config_1g(xpcs);
+	}
+
+	return txgbe_pma_init_done(xpcs);
+}
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 79a34ffb7518..82fe45845ebd 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -228,12 +228,12 @@ static int xpcs_write_vendor(struct dw_xpcs *xpcs, int dev, int reg,
 	return xpcs_write(xpcs, dev, DW_VENDOR | reg, val);
 }
 
-static int xpcs_read_vpcs(struct dw_xpcs *xpcs, int reg)
+int xpcs_read_vpcs(struct dw_xpcs *xpcs, int reg)
 {
 	return xpcs_read_vendor(xpcs, MDIO_MMD_PCS, reg);
 }
 
-static int xpcs_write_vpcs(struct dw_xpcs *xpcs, int reg, u16 val)
+int xpcs_write_vpcs(struct dw_xpcs *xpcs, int reg, u16 val)
 {
 	return xpcs_write_vendor(xpcs, MDIO_MMD_PCS, reg, val);
 }
@@ -826,6 +826,12 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface,
 	if (!compat)
 		return -ENODEV;
 
+	if (xpcs_dev_is_txgbe(xpcs)) {
+		ret = txgbe_xpcs_switch_mode(xpcs, interface);
+		if (ret)
+			return ret;
+	}
+
 	switch (compat->an_mode) {
 	case DW_10GBASER:
 		break;
@@ -1294,8 +1300,6 @@ static struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev,
 
 		xpcs->pcs.ops = &xpcs_phylink_ops;
 		xpcs->pcs.neg_mode = true;
-		if (compat->an_mode == DW_10GBASER)
-			return xpcs;
 
 		if (!xpcs_dev_is_txgbe(xpcs))
 			xpcs->pcs.poll = true;
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 68c6b5a62088..2657138391af 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -15,8 +15,12 @@
 /* VR_XS_PCS */
 #define DW_USXGMII_RST			BIT(10)
 #define DW_USXGMII_EN			BIT(9)
+#define DW_VR_XS_PCS_DIG_CTRL1		0x0000
+#define DW_VR_RST			BIT(15)
 #define DW_VR_XS_PCS_DIG_STS		0x0010
 #define DW_RXFIFO_ERR			GENMASK(6, 5)
+#define DW_PSEQ_ST			GENMASK(4, 2)
+#define DW_PSEQ_ST_GOOD			FIELD_PREP(GENMASK(4, 2), 0x4)
 
 /* SR_MII */
 #define DW_USXGMII_FULL			BIT(8)
@@ -106,6 +110,9 @@
 
 int xpcs_read(struct dw_xpcs *xpcs, int dev, u32 reg);
 int xpcs_write(struct dw_xpcs *xpcs, int dev, u32 reg, u16 val);
+int xpcs_read_vpcs(struct dw_xpcs *xpcs, int reg);
+int xpcs_write_vpcs(struct dw_xpcs *xpcs, int reg, u16 val);
 int nxp_sja1105_sgmii_pma_config(struct dw_xpcs *xpcs);
 int nxp_sja1110_sgmii_pma_config(struct dw_xpcs *xpcs);
 int nxp_sja1110_2500basex_pma_config(struct dw_xpcs *xpcs);
+int txgbe_xpcs_switch_mode(struct dw_xpcs *xpcs, phy_interface_t interface);
diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h
index fb60c7e28623..da8a0708c50f 100644
--- a/include/linux/pcs/pcs-xpcs.h
+++ b/include/linux/pcs/pcs-xpcs.h
@@ -28,6 +28,7 @@ struct dw_xpcs {
 	struct mdio_device *mdiodev;
 	const struct xpcs_id *id;
 	struct phylink_pcs pcs;
+	phy_interface_t interface;
 };
 
 int xpcs_get_an_mode(struct dw_xpcs *xpcs, phy_interface_t interface);
-- 
2.27.0

Re: [PATCH net-next 2/7] net: pcs: xpcs: support to switch mode for Wangxun NICs

[Andrew Lunn]

> +static void txgbe_pma_config_10gbaser(struct dw_xpcs *xpcs)
> +{
> +	int val;
> +
> +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL0, 0x21);
> +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL3, 0);
> +	val = txgbe_read_pma(xpcs, TXGBE_TX_GENCTL1);
> +	val = u16_replace_bits(val, 0x5, TXGBE_TX_GENCTL1_VBOOST_LVL);
> +	txgbe_write_pma(xpcs, TXGBE_TX_GENCTL1, val);
> +	txgbe_write_pma(xpcs, TXGBE_MISC_CTL0, 0xCF00);
> +	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_LD0, 0x549);
> +	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_REF0, 0x29);
> +	txgbe_write_pma(xpcs, TXGBE_TX_RATE_CTL, 0);
> +	txgbe_write_pma(xpcs, TXGBE_RX_RATE_CTL, 0);
> +	txgbe_write_pma(xpcs, TXGBE_TX_GEN_CTL2, 0x300);
> +	txgbe_write_pma(xpcs, TXGBE_RX_GEN_CTL2, 0x300);
> +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL2, 0x600);
> +
> +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL0, 0x45);
> +	val = txgbe_read_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL);
> +	val &= ~TXGBE_RX_EQ_ATTN_LVL0;
> +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL, val);
> +	txgbe_write_pma(xpcs, TXGBE_DFE_TAP_CTL0, 0xBE);

You have a lot of magic numbers above. Please truy to add some
#defines to try to explain what is going on here.

> +	val = txgbe_read_pma(xpcs, TXGBE_AFE_DFE_ENABLE);
> +	val &= ~(TXGBE_DFE_EN_0 | TXGBE_AFE_EN_0);
> +	txgbe_write_pma(xpcs, TXGBE_AFE_DFE_ENABLE, val);
> +	val = txgbe_read_pma(xpcs, TXGBE_RX_EQ_CTL4);
> +	val &= ~TXGBE_RX_EQ_CTL4_CONT_ADAPT0;
> +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL4, val);
> +}
> +
> +static void txgbe_pma_config_1g(struct dw_xpcs *xpcs)
> +{
> +	int val;
> +
> +	val = txgbe_read_pma(xpcs, TXGBE_TX_GENCTL1);
> +	val = u16_replace_bits(val, 0x5, TXGBE_TX_GENCTL1_VBOOST_LVL);
> +	val &= ~TXGBE_TX_GENCTL1_VBOOST_EN0;
> +	txgbe_write_pma(xpcs, TXGBE_TX_GENCTL1, val);
> +	txgbe_write_pma(xpcs, TXGBE_MISC_CTL0, 0xCF00);
> +
> +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL0, 0x7706);
> +	val = txgbe_read_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL);
> +	val &= ~TXGBE_RX_EQ_ATTN_LVL0;
> +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL, val);
> +	txgbe_write_pma(xpcs, TXGBE_DFE_TAP_CTL0, 0);
> +	val = txgbe_read_pma(xpcs, TXGBE_RX_GEN_CTL3);
> +	val = u16_replace_bits(val, 0x4, TXGBE_RX_GEN_CTL3_LOS_TRSHLD0);
> +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL, val);
> +
> +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL0, 0x20);
> +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL3, 0x46);
> +	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_LD0, 0x540);
> +	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_REF0, 0x2A);
> +	txgbe_write_pma(xpcs, TXGBE_AFE_DFE_ENABLE, 0);
> +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL4, 0x10);
> +	txgbe_write_pma(xpcs, TXGBE_TX_RATE_CTL, 0x3);
> +	txgbe_write_pma(xpcs, TXGBE_RX_RATE_CTL, 0x3);
> +	txgbe_write_pma(xpcs, TXGBE_TX_GEN_CTL2, 0x100);
> +	txgbe_write_pma(xpcs, TXGBE_RX_GEN_CTL2, 0x100);
> +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL2, 0x200);
> +}
> +
> +static int txgbe_pcs_poll_power_up(struct dw_xpcs *xpcs)
> +{
> +	int val, ret;
> +
> +	/* Wait xpcs power-up good */
> +	ret = read_poll_timeout(xpcs_read_vpcs, val,
> +				(val & DW_PSEQ_ST) == DW_PSEQ_ST_GOOD,
> +				10000, 1000000, false,
> +				xpcs, DW_VR_XS_PCS_DIG_STS);
> +	if (ret < 0)
> +		pr_err("%s: xpcs power-up timeout\n", __func__);

dev_err(). xpcs->mdiodev->dev.

You want to know which pcs returned an error. Always use dev_err() in
preference to pr_err()

	Andrew

RE: [PATCH net-next 2/7] net: pcs: xpcs: support to switch mode for Wangxun NICs

[Jiawen Wu]

On Wednesday, July 26, 2023 1:32 AM, Andrew Lunn wrote:
> > +static void txgbe_pma_config_10gbaser(struct dw_xpcs *xpcs)
> > +{
> > +	int val;
> > +
> > +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL0, 0x21);
> > +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL3, 0);
> > +	val = txgbe_read_pma(xpcs, TXGBE_TX_GENCTL1);
> > +	val = u16_replace_bits(val, 0x5, TXGBE_TX_GENCTL1_VBOOST_LVL);
> > +	txgbe_write_pma(xpcs, TXGBE_TX_GENCTL1, val);
> > +	txgbe_write_pma(xpcs, TXGBE_MISC_CTL0, 0xCF00);
> > +	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_LD0, 0x549);
> > +	txgbe_write_pma(xpcs, TXGBE_VCO_CAL_REF0, 0x29);
> > +	txgbe_write_pma(xpcs, TXGBE_TX_RATE_CTL, 0);
> > +	txgbe_write_pma(xpcs, TXGBE_RX_RATE_CTL, 0);
> > +	txgbe_write_pma(xpcs, TXGBE_TX_GEN_CTL2, 0x300);
> > +	txgbe_write_pma(xpcs, TXGBE_RX_GEN_CTL2, 0x300);
> > +	txgbe_write_pma(xpcs, TXGBE_MPLLA_CTL2, 0x600);
> > +
> > +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_CTL0, 0x45);
> > +	val = txgbe_read_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL);
> > +	val &= ~TXGBE_RX_EQ_ATTN_LVL0;
> > +	txgbe_write_pma(xpcs, TXGBE_RX_EQ_ATTN_CTL, val);
> > +	txgbe_write_pma(xpcs, TXGBE_DFE_TAP_CTL0, 0xBE);
> 
> You have a lot of magic numbers above. Please truy to add some
> #defines to try to explain what is going on here.

Some registers give only magic numbers in the fields, like a frequency,
bandwidth, etc. And other fields are reserved. Those registers don't
make much sense to define the bits field. But I'll try to add more
useful defines.

[PATCH net-next 3/7] net: pcs: xpcs: add 1000BASE-X AN interrupt support

[Jiawen Wu]

Enable CL37 AN complete interrupt for DW XPCS. It requires to clear the
bit(0) [CL37_ANCMPLT_INTR] of VR_MII_AN_INTR_STS after AN completed.

And there is a quirk for Wangxun devices to enable CL37 AN in backplane
configurations because of the special hardware design.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/pcs/pcs-xpcs.c | 16 ++++++++++++++++
 drivers/net/pcs/pcs-xpcs.h |  3 +++
 2 files changed, 19 insertions(+)

diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index 82fe45845ebd..f38b9241a942 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -740,6 +740,9 @@ static int xpcs_config_aneg_c37_1000basex(struct dw_xpcs *xpcs,
 	int ret, mdio_ctrl, adv;
 	bool changed = 0;
 
+	if (xpcs_dev_is_txgbe(xpcs))
+		xpcs_write_vpcs(xpcs, DW_VR_XS_PCS_DIG_CTRL1, DW_CL37_BP);
+
 	/* According to Chap 7.12, to set 1000BASE-X C37 AN, AN must
 	 * be disabled first:-
 	 * 1) VR_MII_MMD_CTRL Bit(12)[AN_ENABLE] = 0b
@@ -761,6 +764,8 @@ static int xpcs_config_aneg_c37_1000basex(struct dw_xpcs *xpcs,
 		return ret;
 
 	ret &= ~DW_VR_MII_PCS_MODE_MASK;
+	if (!xpcs->pcs.poll)
+		ret |= DW_VR_MII_AN_INTR_EN;
 	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL, ret);
 	if (ret < 0)
 		return ret;
@@ -1014,6 +1019,17 @@ static int xpcs_get_state_c37_1000basex(struct dw_xpcs *xpcs,
 		if (bmsr < 0)
 			return bmsr;
 
+		/* Clear AN complete interrupt */
+		if (!xpcs->pcs.poll) {
+			int an_intr;
+
+			an_intr = xpcs_read(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS);
+			if (an_intr & DW_VR_MII_AN_STS_C37_ANCMPLT_INTR) {
+				an_intr &= ~DW_VR_MII_AN_STS_C37_ANCMPLT_INTR;
+				xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, an_intr);
+			}
+		}
+
 		phylink_mii_c22_pcs_decode_state(state, bmsr, lpa);
 	}
 
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 2657138391af..08a8881614de 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -17,6 +17,7 @@
 #define DW_USXGMII_EN			BIT(9)
 #define DW_VR_XS_PCS_DIG_CTRL1		0x0000
 #define DW_VR_RST			BIT(15)
+#define DW_CL37_BP			BIT(12)
 #define DW_VR_XS_PCS_DIG_STS		0x0010
 #define DW_RXFIFO_ERR			GENMASK(6, 5)
 #define DW_PSEQ_ST			GENMASK(4, 2)
@@ -79,8 +80,10 @@
 #define DW_VR_MII_PCS_MODE_MASK			GENMASK(2, 1)
 #define DW_VR_MII_PCS_MODE_C37_1000BASEX	0x0
 #define DW_VR_MII_PCS_MODE_C37_SGMII		0x2
+#define DW_VR_MII_AN_INTR_EN			BIT(0)
 
 /* VR_MII_AN_INTR_STS */
+#define DW_VR_MII_AN_STS_C37_ANCMPLT_INTR	BIT(0)
 #define DW_VR_MII_AN_STS_C37_ANSGM_FD		BIT(1)
 #define DW_VR_MII_AN_STS_C37_ANSGM_SP_SHIFT	2
 #define DW_VR_MII_AN_STS_C37_ANSGM_SP		GENMASK(3, 2)
-- 
2.27.0

[PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

Wangxun NICs support the connection with SFP to RJ45 module. In this case,
PCS need to be configured in SGMII mode.

Accroding to chapter 6.11.1 "SGMII Auto-Negitiation" of DesignWare Cores
Ethernet PCS (version 3.20a) and custom design manual, do the following
configuration when the interface mode is SGMII.

1. program VR_MII_AN_CTRL bit(3) [TX_CONFIG] = 1b (PHY side SGMII)
2. program VR_MII_AN_CTRL bit(8) [MII_CTRL] = 1b (8-bit MII)
3. program VR_MII_DIG_CTRL1 bit(0) [PHY_MODE_CTRL] = 1b

Also CL37 AN in backplane configurations need to be enabled because of the
special hardware design. Another thing to note is that PMA needs to be
reconfigured before each CL37 AN configuration for SGMII, otherwise AN will
fail, although we don't know why.

On this device, CL37_ANSGM_STS (bit[4:1] of VR_MII_AN_INTR_STS) indicates
the status received from remote link during the auto-negotiation, and
self-clear after the auto-negotiation is complete.
Meanwhile, CL37_ANCMPLT_INTR will be set to 1, to indicate CL37 AN is
complete. So add another way to get the state for CL37 SGMII.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/pcs/pcs-xpcs-wx.c |  5 ++--
 drivers/net/pcs/pcs-xpcs.c    | 46 ++++++++++++++++++++++++++++++++---
 drivers/net/pcs/pcs-xpcs.h    |  6 +++++
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/net/pcs/pcs-xpcs-wx.c b/drivers/net/pcs/pcs-xpcs-wx.c
index 6d1df34958bd..7667924623fa 100644
--- a/drivers/net/pcs/pcs-xpcs-wx.c
+++ b/drivers/net/pcs/pcs-xpcs-wx.c
@@ -143,8 +143,9 @@ static bool txgbe_xpcs_mode_quirk(struct dw_xpcs *xpcs)
 	/* When txgbe do LAN reset, PCS will change to default 10GBASE-R mode */
 	ret = xpcs_read(xpcs, MDIO_MMD_PCS, MDIO_CTRL2);
 	ret &= MDIO_PCS_CTRL2_TYPE;
-	if (ret == MDIO_PCS_CTRL2_10GBR &&
-	    xpcs->interface != PHY_INTERFACE_MODE_10GBASER)
+	if ((ret == MDIO_PCS_CTRL2_10GBR &&
+	     xpcs->interface != PHY_INTERFACE_MODE_10GBASER) ||
+	    xpcs->interface == PHY_INTERFACE_MODE_SGMII)
 		return true;
 
 	return false;
diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c
index f38b9241a942..9f7406b6dd38 100644
--- a/drivers/net/pcs/pcs-xpcs.c
+++ b/drivers/net/pcs/pcs-xpcs.c
@@ -668,7 +668,10 @@ EXPORT_SYMBOL_GPL(xpcs_config_eee);
 static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 				      unsigned int neg_mode)
 {
-	int ret, mdio_ctrl;
+	int ret, mdio_ctrl, tx_conf;
+
+	if (xpcs_dev_is_txgbe(xpcs))
+		xpcs_write_vpcs(xpcs, DW_VR_XS_PCS_DIG_CTRL1, DW_CL37_BP);
 
 	/* For AN for C37 SGMII mode, the settings are :-
 	 * 1) VR_MII_MMD_CTRL Bit(12) [AN_ENABLE] = 0b (Disable SGMII AN in case
@@ -705,9 +708,14 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 	ret |= (DW_VR_MII_PCS_MODE_C37_SGMII <<
 		DW_VR_MII_AN_CTRL_PCS_MODE_SHIFT &
 		DW_VR_MII_PCS_MODE_MASK);
-	ret |= (DW_VR_MII_TX_CONFIG_MAC_SIDE_SGMII <<
-		DW_VR_MII_AN_CTRL_TX_CONFIG_SHIFT &
-		DW_VR_MII_TX_CONFIG_MASK);
+	if (xpcs_dev_is_txgbe(xpcs)) {
+		ret |= DW_VR_MII_AN_CTRL_8BIT;
+		tx_conf = DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII;
+	} else {
+		tx_conf = DW_VR_MII_TX_CONFIG_MAC_SIDE_SGMII;
+	}
+	ret |= tx_conf << DW_VR_MII_AN_CTRL_TX_CONFIG_SHIFT &
+		DW_VR_MII_TX_CONFIG_MASK;
 	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_CTRL, ret);
 	if (ret < 0)
 		return ret;
@@ -721,6 +729,9 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
 	else
 		ret &= ~DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW;
 
+	if (xpcs_dev_is_txgbe(xpcs))
+		ret |= DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL;
+
 	ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_DIG_CTRL1, ret);
 	if (ret < 0)
 		return ret;
@@ -996,6 +1007,33 @@ static int xpcs_get_state_c37_sgmii(struct dw_xpcs *xpcs,
 			state->duplex = DUPLEX_FULL;
 		else
 			state->duplex = DUPLEX_HALF;
+	} else if (ret == DW_VR_MII_AN_STS_C37_ANCMPLT_INTR) {
+		int speed, duplex;
+
+		state->link = true;
+
+		speed = xpcs_read(xpcs, MDIO_MMD_VEND2, MDIO_CTRL1);
+		if (speed < 0)
+			return speed;
+
+		speed &= SGMII_SPEED_SS13 | SGMII_SPEED_SS6;
+		if (speed == SGMII_SPEED_SS6)
+			state->speed = SPEED_1000;
+		else if (speed == SGMII_SPEED_SS13)
+			state->speed = SPEED_100;
+		else if (speed == 0)
+			state->speed = SPEED_10;
+
+		duplex = xpcs_read(xpcs, MDIO_MMD_VEND2, MII_ADVERTISE);
+		if (duplex < 0)
+			return duplex;
+
+		if (duplex & DW_FULL_DUPLEX)
+			state->duplex = DUPLEX_FULL;
+		else if (duplex & DW_HALF_DUPLEX)
+			state->duplex = DUPLEX_HALF;
+
+		xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_AN_INTR_STS, 0);
 	}
 
 	return 0;
diff --git a/drivers/net/pcs/pcs-xpcs.h b/drivers/net/pcs/pcs-xpcs.h
index 08a8881614de..36a1786a2853 100644
--- a/drivers/net/pcs/pcs-xpcs.h
+++ b/drivers/net/pcs/pcs-xpcs.h
@@ -66,12 +66,14 @@
 
 /* VR_MII_DIG_CTRL1 */
 #define DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW		BIT(9)
+#define DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL	BIT(0)
 
 /* VR_MII_DIG_CTRL2 */
 #define DW_VR_MII_DIG_CTRL2_TX_POL_INV		BIT(4)
 #define DW_VR_MII_DIG_CTRL2_RX_POL_INV		BIT(0)
 
 /* VR_MII_AN_CTRL */
+#define DW_VR_MII_AN_CTRL_8BIT			BIT(8)
 #define DW_VR_MII_AN_CTRL_TX_CONFIG_SHIFT	3
 #define DW_VR_MII_TX_CONFIG_MASK		BIT(3)
 #define DW_VR_MII_TX_CONFIG_PHY_SIDE_SGMII	0x1
@@ -97,6 +99,10 @@
 #define SGMII_SPEED_SS13		BIT(13)	/* SGMII speed along with SS6 */
 #define SGMII_SPEED_SS6			BIT(6)	/* SGMII speed along with SS13 */
 
+/* SR MII MMD AN Advertisement defines */
+#define DW_HALF_DUPLEX			BIT(6)
+#define DW_FULL_DUPLEX			BIT(5)
+
 /* VR MII EEE Control 0 defines */
 #define DW_VR_MII_EEE_LTX_EN			BIT(0)  /* LPI Tx Enable */
 #define DW_VR_MII_EEE_LRX_EN			BIT(1)  /* LPI Rx Enable */
-- 
2.27.0

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Russell King (Oracle)]

On Mon, Jul 24, 2023 at 06:23:38PM +0800, Jiawen Wu wrote:
> Wangxun NICs support the connection with SFP to RJ45 module. In this case,
> PCS need to be configured in SGMII mode.
> 
> Accroding to chapter 6.11.1 "SGMII Auto-Negitiation" of DesignWare Cores
> Ethernet PCS (version 3.20a) and custom design manual, do the following
> configuration when the interface mode is SGMII.
> 
> 1. program VR_MII_AN_CTRL bit(3) [TX_CONFIG] = 1b (PHY side SGMII)
> 2. program VR_MII_AN_CTRL bit(8) [MII_CTRL] = 1b (8-bit MII)
> 3. program VR_MII_DIG_CTRL1 bit(0) [PHY_MODE_CTRL] = 1b

I'm confused by "PHY side SGMII" - what does this mean for the
transmitted 16-bit configuration word? Does it mean that _this_ side
is acting as if it were a PHY?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

On Monday, July 24, 2023 6:35 PM, Russell King (Oracle) wrote:
> On Mon, Jul 24, 2023 at 06:23:38PM +0800, Jiawen Wu wrote:
> > Wangxun NICs support the connection with SFP to RJ45 module. In this case,
> > PCS need to be configured in SGMII mode.
> >
> > Accroding to chapter 6.11.1 "SGMII Auto-Negitiation" of DesignWare Cores
> > Ethernet PCS (version 3.20a) and custom design manual, do the following
> > configuration when the interface mode is SGMII.
> >
> > 1. program VR_MII_AN_CTRL bit(3) [TX_CONFIG] = 1b (PHY side SGMII)
> > 2. program VR_MII_AN_CTRL bit(8) [MII_CTRL] = 1b (8-bit MII)
> > 3. program VR_MII_DIG_CTRL1 bit(0) [PHY_MODE_CTRL] = 1b
> 
> I'm confused by "PHY side SGMII" - what does this mean for the
> transmitted 16-bit configuration word? Does it mean that _this_ side
> is acting as if it were a PHY?

I'm not sure, because the datasheet doesn't explicitly describe it. In this
case, the PHY is integrated in the SFP to RJ45 module. From my point of
view, TX control occurs on the PHY side. So program it as PHY side SGMII.

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Russell King (Oracle)]

On Tue, Jul 25, 2023 at 10:05:05AM +0800, Jiawen Wu wrote:
> On Monday, July 24, 2023 6:35 PM, Russell King (Oracle) wrote:
> > On Mon, Jul 24, 2023 at 06:23:38PM +0800, Jiawen Wu wrote:
> > > Wangxun NICs support the connection with SFP to RJ45 module. In this case,
> > > PCS need to be configured in SGMII mode.
> > >
> > > Accroding to chapter 6.11.1 "SGMII Auto-Negitiation" of DesignWare Cores
> > > Ethernet PCS (version 3.20a) and custom design manual, do the following
> > > configuration when the interface mode is SGMII.
> > >
> > > 1. program VR_MII_AN_CTRL bit(3) [TX_CONFIG] = 1b (PHY side SGMII)
> > > 2. program VR_MII_AN_CTRL bit(8) [MII_CTRL] = 1b (8-bit MII)
> > > 3. program VR_MII_DIG_CTRL1 bit(0) [PHY_MODE_CTRL] = 1b
> > 
> > I'm confused by "PHY side SGMII" - what does this mean for the
> > transmitted 16-bit configuration word? Does it mean that _this_ side
> > is acting as if it were a PHY?
> 
> I'm not sure, because the datasheet doesn't explicitly describe it. In this
> case, the PHY is integrated in the SFP to RJ45 module. From my point of
> view, TX control occurs on the PHY side. So program it as PHY side SGMII.

Let me ask the question a different way. Would you use "PHY side SGMII"
if the PHY was directly connected to the PCS with SGMII?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

On Tuesday, July 25, 2023 3:49 PM, Russell King (Oracle) wrote:
> On Tue, Jul 25, 2023 at 10:05:05AM +0800, Jiawen Wu wrote:
> > On Monday, July 24, 2023 6:35 PM, Russell King (Oracle) wrote:
> > > On Mon, Jul 24, 2023 at 06:23:38PM +0800, Jiawen Wu wrote:
> > > > Wangxun NICs support the connection with SFP to RJ45 module. In this case,
> > > > PCS need to be configured in SGMII mode.
> > > >
> > > > Accroding to chapter 6.11.1 "SGMII Auto-Negitiation" of DesignWare Cores
> > > > Ethernet PCS (version 3.20a) and custom design manual, do the following
> > > > configuration when the interface mode is SGMII.
> > > >
> > > > 1. program VR_MII_AN_CTRL bit(3) [TX_CONFIG] = 1b (PHY side SGMII)
> > > > 2. program VR_MII_AN_CTRL bit(8) [MII_CTRL] = 1b (8-bit MII)
> > > > 3. program VR_MII_DIG_CTRL1 bit(0) [PHY_MODE_CTRL] = 1b
> > >
> > > I'm confused by "PHY side SGMII" - what does this mean for the
> > > transmitted 16-bit configuration word? Does it mean that _this_ side
> > > is acting as if it were a PHY?
> >
> > I'm not sure, because the datasheet doesn't explicitly describe it. In this
> > case, the PHY is integrated in the SFP to RJ45 module. From my point of
> > view, TX control occurs on the PHY side. So program it as PHY side SGMII.
> 
> Let me ask the question a different way. Would you use "PHY side SGMII"
> if the PHY was directly connected to the PCS with SGMII?

The information obtained from the IC designer is that "PHY/MAC side SGMII"
is configured by experimentation. For these different kinds of NICs:
1) fiber + SFP-RJ45 module: PHY side SGMII
2) copper (pcs + external PHY): MAC side SGMII
3) backplane: PHY side SGMII

Backplane is not supported in the driver yet. If consider the "side" as "acting"
point, case 1 (this patch implement) seems more like "MAC side SGMII". But
in practice, it won't work.

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Russell King (Oracle)]

On Tue, Jul 25, 2023 at 05:50:36PM +0800, Jiawen Wu wrote:
> On Tuesday, July 25, 2023 3:49 PM, Russell King (Oracle) wrote:
> > On Tue, Jul 25, 2023 at 10:05:05AM +0800, Jiawen Wu wrote:
> > > On Monday, July 24, 2023 6:35 PM, Russell King (Oracle) wrote:
> > > > On Mon, Jul 24, 2023 at 06:23:38PM +0800, Jiawen Wu wrote:
> > > > > Wangxun NICs support the connection with SFP to RJ45 module. In this case,
> > > > > PCS need to be configured in SGMII mode.
> > > > >
> > > > > Accroding to chapter 6.11.1 "SGMII Auto-Negitiation" of DesignWare Cores
> > > > > Ethernet PCS (version 3.20a) and custom design manual, do the following
> > > > > configuration when the interface mode is SGMII.
> > > > >
> > > > > 1. program VR_MII_AN_CTRL bit(3) [TX_CONFIG] = 1b (PHY side SGMII)
> > > > > 2. program VR_MII_AN_CTRL bit(8) [MII_CTRL] = 1b (8-bit MII)
> > > > > 3. program VR_MII_DIG_CTRL1 bit(0) [PHY_MODE_CTRL] = 1b
> > > >
> > > > I'm confused by "PHY side SGMII" - what does this mean for the
> > > > transmitted 16-bit configuration word? Does it mean that _this_ side
> > > > is acting as if it were a PHY?
> > >
> > > I'm not sure, because the datasheet doesn't explicitly describe it. In this
> > > case, the PHY is integrated in the SFP to RJ45 module. From my point of
> > > view, TX control occurs on the PHY side. So program it as PHY side SGMII.
> > 
> > Let me ask the question a different way. Would you use "PHY side SGMII"
> > if the PHY was directly connected to the PCS with SGMII?
> 
> The information obtained from the IC designer is that "PHY/MAC side SGMII"
> is configured by experimentation. For these different kinds of NICs:
> 1) fiber + SFP-RJ45 module: PHY side SGMII
> 2) copper (pcs + external PHY): MAC side SGMII

This makes no sense. a PHY on a RJ45 SFP module is just the same as a
PHY integrated into a board with the MAC.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Russell King (Oracle)]

On Tue, Jul 25, 2023 at 10:58:25AM +0100, Russell King (Oracle) wrote:
> > The information obtained from the IC designer is that "PHY/MAC side SGMII"
> > is configured by experimentation. For these different kinds of NICs:
> > 1) fiber + SFP-RJ45 module: PHY side SGMII
> > 2) copper (pcs + external PHY): MAC side SGMII
> 
> This makes no sense. a PHY on a RJ45 SFP module is just the same as a
> PHY integrated into a board with the MAC.


MAC ---- PCS <----- sgmii -----> PHY (whether on a board or SFP)

Control word flow:
             <------------------ link, speed, duplex
	     ------------------> acknowledge (value = 0x4001)

Sometimes, it's possible to connect two MACs/PCSs together:

MAC ---- PCS <----- sgmii -----> PCS ---- MAC

and in this case, one PCS would need to be configured in "MAC" mode
and the other would need to be configured in "PHY" mode because SGMII
is fundamentally asymmetric.

Here is the definition of the control word sent by either end:

Bit	MAC->PHY	PHY->MAC
15	0: Reserved	Link status, 1 = link up
14	1: Acknowledge	Reserved for AN acknowledge
13	0: Reserved	0: Reserved
12	0: Reserved	Duplex mode 1 = full, 0 = half
11:10	0: Reserved	Speed 11 = Reserved 10=1G, 01=100M, 00=10M
9:1	0: Reserved	0: Reserved
0	1		1

So my guess would be that "PHY side SGMII" means the device generates
the "PHY->MAC" format word whereas "MAC side SGMII" generates the
"MAC->PHY" format word - and it's the latter that you want to be using
both for Copper SFPs, which are no different from PHYs integrated onto
the board connected with SGMII.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

On Tuesday, July 25, 2023 6:08 PM, Russell King (Oracle) wrote:
> On Tue, Jul 25, 2023 at 10:58:25AM +0100, Russell King (Oracle) wrote:
> > > The information obtained from the IC designer is that "PHY/MAC side SGMII"
> > > is configured by experimentation. For these different kinds of NICs:
> > > 1) fiber + SFP-RJ45 module: PHY side SGMII
> > > 2) copper (pcs + external PHY): MAC side SGMII
> >
> > This makes no sense. a PHY on a RJ45 SFP module is just the same as a
> > PHY integrated into a board with the MAC.
> 
> 
> MAC ---- PCS <----- sgmii -----> PHY (whether on a board or SFP)
> 
> Control word flow:
>              <------------------ link, speed, duplex
> 	     ------------------> acknowledge (value = 0x4001)
> 
> Sometimes, it's possible to connect two MACs/PCSs together:
> 
> MAC ---- PCS <----- sgmii -----> PCS ---- MAC
> 
> and in this case, one PCS would need to be configured in "MAC" mode
> and the other would need to be configured in "PHY" mode because SGMII
> is fundamentally asymmetric.
> 
> Here is the definition of the control word sent by either end:
> 
> Bit	MAC->PHY	PHY->MAC
> 15	0: Reserved	Link status, 1 = link up
> 14	1: Acknowledge	Reserved for AN acknowledge
> 13	0: Reserved	0: Reserved
> 12	0: Reserved	Duplex mode 1 = full, 0 = half
> 11:10	0: Reserved	Speed 11 = Reserved 10=1G, 01=100M, 00=10M
> 9:1	0: Reserved	0: Reserved
> 0	1		1
> 
> So my guess would be that "PHY side SGMII" means the device generates
> the "PHY->MAC" format word whereas "MAC side SGMII" generates the
> "MAC->PHY" format word - and it's the latter that you want to be using
> both for Copper SFPs, which are no different from PHYs integrated onto
> the board connected with SGMII.

Thanks for the detailed explanation, I can understand it.

So I need to find out why config it in MAC mode doesn't work. When I config
it in MAC mode, PHY would not change state to callback the xpcs_get_state().
I dump the PCS register through the tool at this time, VR_MII_AN_INTR_STS
is not always the same value, sometimes AN complete, sometimes not.

I'm not sure if this is related to the anomaly, log often shows:
"i2c_designware i2c_designware.1024: timeout in disabling adapter"

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

On Tuesday, July 25, 2023 6:08 PM, Russell King (Oracle) wrote:
> On Tue, Jul 25, 2023 at 10:58:25AM +0100, Russell King (Oracle) wrote:
> > > The information obtained from the IC designer is that "PHY/MAC side SGMII"
> > > is configured by experimentation. For these different kinds of NICs:
> > > 1) fiber + SFP-RJ45 module: PHY side SGMII
> > > 2) copper (pcs + external PHY): MAC side SGMII
> >
> > This makes no sense. a PHY on a RJ45 SFP module is just the same as a
> > PHY integrated into a board with the MAC.
> 
> 
> MAC ---- PCS <----- sgmii -----> PHY (whether on a board or SFP)
> 
> Control word flow:
>              <------------------ link, speed, duplex
> 	     ------------------> acknowledge (value = 0x4001)
> 
> Sometimes, it's possible to connect two MACs/PCSs together:
> 
> MAC ---- PCS <----- sgmii -----> PCS ---- MAC
> 
> and in this case, one PCS would need to be configured in "MAC" mode
> and the other would need to be configured in "PHY" mode because SGMII
> is fundamentally asymmetric.
> 
> Here is the definition of the control word sent by either end:
> 
> Bit	MAC->PHY	PHY->MAC
> 15	0: Reserved	Link status, 1 = link up
> 14	1: Acknowledge	Reserved for AN acknowledge
> 13	0: Reserved	0: Reserved
> 12	0: Reserved	Duplex mode 1 = full, 0 = half
> 11:10	0: Reserved	Speed 11 = Reserved 10=1G, 01=100M, 00=10M
> 9:1	0: Reserved	0: Reserved
> 0	1		1
> 
> So my guess would be that "PHY side SGMII" means the device generates
> the "PHY->MAC" format word whereas "MAC side SGMII" generates the
> "MAC->PHY" format word - and it's the latter that you want to be using
> both for Copper SFPs, which are no different from PHYs integrated onto
> the board connected with SGMII.

There is a question about I2C MII read ops. I see that PHY in SFP-RJ45 module
is read by i2c_mii_read_default_c22(), but it limits the msgs[0].len=1.

A description in  the SFP-RJ45 datasheet shows:
The registers are accessible through the 2-wire serial CMOS EEPROM protocol
of the ATMEL AT24C01A or equivalent. The address of the PHY is 1010110x,
where x is the R/W bit. Each register's address is 000yyyyy, where yyyyy is the
binary equivalent of the register number. Write and read operations must send
or receive 16 bits of data, so the "multi-page" access protocol must be used.

So the PHY register address should be written twice: first high 8 bits, second low
8 bits. to read the register value.

Is there a problem with which driver?

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Andrew Lunn]

> There is a question about I2C MII read ops. I see that PHY in SFP-RJ45 module
> is read by i2c_mii_read_default_c22(), but it limits the msgs[0].len=1.
> 
> A description in  the SFP-RJ45 datasheet shows:

Please give a link to this document.

The problem with copper PHYs embedded inside SFPs is that there is no
standardised protocol to talk to them. Each PHY vendor does their own
thing. At the moment, two different protocols are supported, ROLLBALL
and the default protocol. It might be another protocol needs to be
added to support the SFP you are testing with. So we need to see the
protocol specification.

      Andrew

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

On Friday, July 28, 2023 6:24 PM, : Andrew Lunn wrote:
> > There is a question about I2C MII read ops. I see that PHY in SFP-RJ45 module
> > is read by i2c_mii_read_default_c22(), but it limits the msgs[0].len=1.
> >
> > A description in  the SFP-RJ45 datasheet shows:
> 
> Please give a link to this document.

https://www.alldatasheet.com/datasheet-pdf/pdf/465165/AVAGO/ABCU-5710RZ.html

> 
> The problem with copper PHYs embedded inside SFPs is that there is no
> standardised protocol to talk to them. Each PHY vendor does their own
> thing. At the moment, two different protocols are supported, ROLLBALL
> and the default protocol. It might be another protocol needs to be
> added to support the SFP you are testing with. So we need to see the
> protocol specification.
> 
>       Andrew
> 

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Russell King (Oracle)]

On Fri, Jul 28, 2023 at 06:11:51PM +0800, Jiawen Wu wrote:
> On Tuesday, July 25, 2023 6:08 PM, Russell King (Oracle) wrote:
> > On Tue, Jul 25, 2023 at 10:58:25AM +0100, Russell King (Oracle) wrote:
> > > > The information obtained from the IC designer is that "PHY/MAC side SGMII"
> > > > is configured by experimentation. For these different kinds of NICs:
> > > > 1) fiber + SFP-RJ45 module: PHY side SGMII
> > > > 2) copper (pcs + external PHY): MAC side SGMII
> > >
> > > This makes no sense. a PHY on a RJ45 SFP module is just the same as a
> > > PHY integrated into a board with the MAC.
> > 
> > 
> > MAC ---- PCS <----- sgmii -----> PHY (whether on a board or SFP)
> > 
> > Control word flow:
> >              <------------------ link, speed, duplex
> > 	     ------------------> acknowledge (value = 0x4001)
> > 
> > Sometimes, it's possible to connect two MACs/PCSs together:
> > 
> > MAC ---- PCS <----- sgmii -----> PCS ---- MAC
> > 
> > and in this case, one PCS would need to be configured in "MAC" mode
> > and the other would need to be configured in "PHY" mode because SGMII
> > is fundamentally asymmetric.
> > 
> > Here is the definition of the control word sent by either end:
> > 
> > Bit	MAC->PHY	PHY->MAC
> > 15	0: Reserved	Link status, 1 = link up
> > 14	1: Acknowledge	Reserved for AN acknowledge
> > 13	0: Reserved	0: Reserved
> > 12	0: Reserved	Duplex mode 1 = full, 0 = half
> > 11:10	0: Reserved	Speed 11 = Reserved 10=1G, 01=100M, 00=10M
> > 9:1	0: Reserved	0: Reserved
> > 0	1		1
> > 
> > So my guess would be that "PHY side SGMII" means the device generates
> > the "PHY->MAC" format word whereas "MAC side SGMII" generates the
> > "MAC->PHY" format word - and it's the latter that you want to be using
> > both for Copper SFPs, which are no different from PHYs integrated onto
> > the board connected with SGMII.
> 
> There is a question about I2C MII read ops. I see that PHY in SFP-RJ45 module
> is read by i2c_mii_read_default_c22(), but it limits the msgs[0].len=1.
> 
> A description in  the SFP-RJ45 datasheet shows:
> The registers are accessible through the 2-wire serial CMOS EEPROM protocol
> of the ATMEL AT24C01A or equivalent. The address of the PHY is 1010110x,
> where x is the R/W bit. Each register's address is 000yyyyy, where yyyyy is the
> binary equivalent of the register number. Write and read operations must send
> or receive 16 bits of data, so the "multi-page" access protocol must be used.
> 
> So the PHY register address should be written twice: first high 8 bits, second low
> 8 bits. to read the register value.
> 
> Is there a problem with which driver?

No there isn't, and it conforms with the above.

A read looks like this:

      Address  Data                   Address  Data     Data
Start 10101100 000yyyyy RepeatedStart 10101101 DDDDDDDD DDDDDDDD Stop
                      or Stop followed
		          by Start

The terms "Address" and "Data" here are as per the I²C specification.
You will notice that the first part has one byte of address and *one*
byte of data to convey the register address. This is what the "1" you
are referring to above is for.

For completness, a write looks like this:

      Address  Data     Data     Data
Start 10101100 000yyyyy DDDDDDDD DDDDDDDD Stop

Essentially, in all cases, when 0x56 is addressed with the data
direction in write mode, the very first byte is _always_ the register
address and the remainder contain the data. When the data direction is
in read mode, the bytes are always data.

The description you quote above is poor because it doesn't make it
clear whether "read" and "write" apply to the bus transactions or to
the device operations. However, I can assure you that what is
implemented is correct, since this is the standard small 24xx memory
device protocol, and I've been programming that on various
microcontrollers and such like for the last 30 years.

Are you seeing a problem with the data read or written to the PHY?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

> No there isn't, and it conforms with the above.
> 
> A read looks like this:
> 
>       Address  Data                   Address  Data     Data
> Start 10101100 000yyyyy RepeatedStart 10101101 DDDDDDDD DDDDDDDD Stop
>                       or Stop followed
> 		          by Start
> 
> The terms "Address" and "Data" here are as per the I²C specification.
> You will notice that the first part has one byte of address and *one*
> byte of data to convey the register address. This is what the "1" you
> are referring to above is for.
> 
> For completness, a write looks like this:
> 
>       Address  Data     Data     Data
> Start 10101100 000yyyyy DDDDDDDD DDDDDDDD Stop
> 
> Essentially, in all cases, when 0x56 is addressed with the data
> direction in write mode, the very first byte is _always_ the register
> address and the remainder contain the data. When the data direction is
> in read mode, the bytes are always data.
> 
> The description you quote above is poor because it doesn't make it
> clear whether "read" and "write" apply to the bus transactions or to
> the device operations. However, I can assure you that what is
> implemented is correct, since this is the standard small 24xx memory
> device protocol, and I've been programming that on various
> microcontrollers and such like for the last 30 years.
> 
> Are you seeing a problem with the data read or written to the PHY?

You are right, I misunderstood it. :(

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

> No there isn't, and it conforms with the above.
> 
> A read looks like this:
> 
>       Address  Data                   Address  Data     Data
> Start 10101100 000yyyyy RepeatedStart 10101101 DDDDDDDD DDDDDDDD Stop
>                       or Stop followed
> 		          by Start
> 
> The terms "Address" and "Data" here are as per the I²C specification.
> You will notice that the first part has one byte of address and *one*
> byte of data to convey the register address. This is what the "1" you
> are referring to above is for.
> 
> For completness, a write looks like this:
> 
>       Address  Data     Data     Data
> Start 10101100 000yyyyy DDDDDDDD DDDDDDDD Stop
> 
> Essentially, in all cases, when 0x56 is addressed with the data
> direction in write mode, the very first byte is _always_ the register
> address and the remainder contain the data. When the data direction is
> in read mode, the bytes are always data.
> 
> The description you quote above is poor because it doesn't make it
> clear whether "read" and "write" apply to the bus transactions or to
> the device operations. However, I can assure you that what is
> implemented is correct, since this is the standard small 24xx memory
> device protocol, and I've been programming that on various
> microcontrollers and such like for the last 30 years.
> 
> Are you seeing a problem with the data read or written to the PHY?

Hi Russell,

I really don't know how to deal with "MAC side SGMII", could you please
help me?

From the test results, when I config PCS in "PHY side SGMII", the link status
of PHY in copper SFP is read by I2C after AN complete. Then PHY's link up
status is informed to PHYLINK, then PCS will check its status. But when I just
change PCS to "MAC side SGMII", I2C will keep reading timeouts since AN
complete. I checked the register of PCS to confirm AN complete, but PHY's
link status would never be updated in PHYLINK.

It's kind of weird to me, how does the configuration of PCS relate to I2C?

Thanks!

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Russell King (Oracle)]

On Thu, Aug 03, 2023 at 10:20:22AM +0800, Jiawen Wu wrote:
> > No there isn't, and it conforms with the above.
> > 
> > A read looks like this:
> > 
> >       Address  Data                   Address  Data     Data
> > Start 10101100 000yyyyy RepeatedStart 10101101 DDDDDDDD DDDDDDDD Stop
> >                       or Stop followed
> > 		          by Start
> > 
> > The terms "Address" and "Data" here are as per the I²C specification.
> > You will notice that the first part has one byte of address and *one*
> > byte of data to convey the register address. This is what the "1" you
> > are referring to above is for.
> > 
> > For completness, a write looks like this:
> > 
> >       Address  Data     Data     Data
> > Start 10101100 000yyyyy DDDDDDDD DDDDDDDD Stop
> > 
> > Essentially, in all cases, when 0x56 is addressed with the data
> > direction in write mode, the very first byte is _always_ the register
> > address and the remainder contain the data. When the data direction is
> > in read mode, the bytes are always data.
> > 
> > The description you quote above is poor because it doesn't make it
> > clear whether "read" and "write" apply to the bus transactions or to
> > the device operations. However, I can assure you that what is
> > implemented is correct, since this is the standard small 24xx memory
> > device protocol, and I've been programming that on various
> > microcontrollers and such like for the last 30 years.
> > 
> > Are you seeing a problem with the data read or written to the PHY?
> 
> Hi Russell,
> 
> I really don't know how to deal with "MAC side SGMII", could you please
> help me?
> 
> From the test results, when I config PCS in "PHY side SGMII", the link status
> of PHY in copper SFP is read by I2C after AN complete. Then PHY's link up
> status is informed to PHYLINK, then PCS will check its status. But when I just
> change PCS to "MAC side SGMII", I2C will keep reading timeouts since AN
> complete. I checked the register of PCS to confirm AN complete, but PHY's
> link status would never be updated in PHYLINK.

I don't understand what is going on here either - but what I do know
is that there is _zero_ difference as far as the network link is
concerned between an on-board PHY using SGMII to the MAC/PCS and a SFP
with a PHY using SGMII.

In both situations the PHY behaves the same - it presents a PHY-side
SGMII interface, so it sends to the MAC/PCS the speed and duplex
settings, and expects the MAC/PCS to acknowledge them.

The name "MAC side SGMII" suggests that this mode provides the
acknowledgement, whereas "PHY side SGMII" suggests that this mode
provides a speed and duplex.

Given all this, using "PHY side SGMII" with a SFP, and "MAC side
SGMII" for an on-board PHY just seems utterly wrong - and I can't
make head nor tail of it.

> It's kind of weird to me, how does the configuration of PCS relate to I2C?

I2C is just the access method for PHYs on SFPs - because there are
no MDIO bus pins on SFP modules, only I2C pins mainly for accessing
the identification EEPROM and diagnostics, but many copper SFPs have
a way to access the PHY.

I2C is transparent as far as phylib is concerned - the mdio-i2c
driver makes the PHY "appear" as if it is on a conventional MDIO
bus.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Jiawen Wu]

On Thursday, August 3, 2023 7:11 PM, Russell King (Oracle) wrote:
> On Thu, Aug 03, 2023 at 10:20:22AM +0800, Jiawen Wu wrote:
> > > No there isn't, and it conforms with the above.
> > >
> > > A read looks like this:
> > >
> > >       Address  Data                   Address  Data     Data
> > > Start 10101100 000yyyyy RepeatedStart 10101101 DDDDDDDD DDDDDDDD Stop
> > >                       or Stop followed
> > > 		          by Start
> > >
> > > The terms "Address" and "Data" here are as per the I²C specification.
> > > You will notice that the first part has one byte of address and *one*
> > > byte of data to convey the register address. This is what the "1" you
> > > are referring to above is for.
> > >
> > > For completness, a write looks like this:
> > >
> > >       Address  Data     Data     Data
> > > Start 10101100 000yyyyy DDDDDDDD DDDDDDDD Stop
> > >
> > > Essentially, in all cases, when 0x56 is addressed with the data
> > > direction in write mode, the very first byte is _always_ the register
> > > address and the remainder contain the data. When the data direction is
> > > in read mode, the bytes are always data.
> > >
> > > The description you quote above is poor because it doesn't make it
> > > clear whether "read" and "write" apply to the bus transactions or to
> > > the device operations. However, I can assure you that what is
> > > implemented is correct, since this is the standard small 24xx memory
> > > device protocol, and I've been programming that on various
> > > microcontrollers and such like for the last 30 years.
> > >
> > > Are you seeing a problem with the data read or written to the PHY?
> >
> > Hi Russell,
> >
> > I really don't know how to deal with "MAC side SGMII", could you please
> > help me?
> >
> > From the test results, when I config PCS in "PHY side SGMII", the link status
> > of PHY in copper SFP is read by I2C after AN complete. Then PHY's link up
> > status is informed to PHYLINK, then PCS will check its status. But when I just
> > change PCS to "MAC side SGMII", I2C will keep reading timeouts since AN
> > complete. I checked the register of PCS to confirm AN complete, but PHY's
> > link status would never be updated in PHYLINK.
> 
> I don't understand what is going on here either - but what I do know
> is that there is _zero_ difference as far as the network link is
> concerned between an on-board PHY using SGMII to the MAC/PCS and a SFP
> with a PHY using SGMII.
> 
> In both situations the PHY behaves the same - it presents a PHY-side
> SGMII interface, so it sends to the MAC/PCS the speed and duplex
> settings, and expects the MAC/PCS to acknowledge them.
> 
> The name "MAC side SGMII" suggests that this mode provides the
> acknowledgement, whereas "PHY side SGMII" suggests that this mode
> provides a speed and duplex.
> 
> Given all this, using "PHY side SGMII" with a SFP, and "MAC side
> SGMII" for an on-board PHY just seems utterly wrong - and I can't
> make head nor tail of it.

Since no reasonable explanation can be given, can we assume that there is a
design flaw in the hardware? Although it's not clear to the designers...

> 
> > It's kind of weird to me, how does the configuration of PCS relate to I2C?
> 
> I2C is just the access method for PHYs on SFPs - because there are
> no MDIO bus pins on SFP modules, only I2C pins mainly for accessing
> the identification EEPROM and diagnostics, but many copper SFPs have
> a way to access the PHY.
> 
> I2C is transparent as far as phylib is concerned - the mdio-i2c
> driver makes the PHY "appear" as if it is on a conventional MDIO
> bus.
 

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Andrew Lunn]

> +	if (xpcs_dev_is_txgbe(xpcs))
> +		xpcs_write_vpcs(xpcs, DW_VR_XS_PCS_DIG_CTRL1, DW_CL37_BP);

xpcs_dev_is_txgbe(xpcs) is being used quite a bit, and its not cheep
since it is a memcmp(). So suggest you do it once and add a quirk flag
to the xpcs structure.

	Andrew

Re: [PATCH net-next 4/7] net: pcs: xpcs: adapt Wangxun NICs for SGMII mode

[Simon Horman]

On Mon, Jul 24, 2023 at 06:23:38PM +0800, Jiawen Wu wrote:
> Wangxun NICs support the connection with SFP to RJ45 module. In this case,
> PCS need to be configured in SGMII mode.
> 
> Accroding to chapter 6.11.1 "SGMII Auto-Negitiation" of DesignWare Cores

nit: Accroding -> According

...

[PATCH net-next 5/7] net: txgbe: support switching mode to 1000BASE-X and SGMII

[Jiawen Wu]

Disable data path before PCS VR reset while switching PCS mode, to prevent
the blocking of data path. Enable AN interrupt for CL37 auto-negotiation.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  2 ++
 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c | 28 +++++++++++++++++++
 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h |  2 ++
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 22 +++++++++++++--
 4 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 1de88a33a698..50b92cfb46a0 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -205,6 +205,8 @@
 #define WX_TSC_CTL                   0x1D000
 #define WX_TSC_CTL_TX_DIS            BIT(1)
 #define WX_TSC_CTL_TSEC_DIS          BIT(0)
+#define WX_TSC_ST                    0x1D004
+#define WX_TSC_ST_SECTX_RDY          BIT(0)
 #define WX_TSC_BUF_AE                0x1D00C
 #define WX_TSC_BUF_AE_THR            GENMASK(9, 0)
 
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
index 6e130d1f7a7b..90168aab11ae 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
@@ -13,6 +13,34 @@
 #include "txgbe_type.h"
 #include "txgbe_hw.h"
 
+/**
+ *  txgbe_disable_sec_tx_path - Stops the transmit data path
+ *  @wx: pointer to hardware structure
+ *
+ *  Stops the transmit data path and waits for the HW to internally empty
+ *  the tx security block
+ **/
+int txgbe_disable_sec_tx_path(struct wx *wx)
+{
+	int val;
+
+	wr32m(wx, WX_TSC_CTL, WX_TSC_CTL_TX_DIS, WX_TSC_CTL_TX_DIS);
+	return read_poll_timeout(rd32, val, val & WX_TSC_ST_SECTX_RDY,
+				 1000, 20000, false, wx, WX_TSC_ST);
+}
+
+/**
+ *  txgbe_enable_sec_tx_path - Enables the transmit data path
+ *  @wx: pointer to hardware structure
+ *
+ *  Enables the transmit data path.
+ **/
+void txgbe_enable_sec_tx_path(struct wx *wx)
+{
+	wr32m(wx, WX_TSC_CTL, WX_TSC_CTL_TX_DIS, 0);
+	WX_WRITE_FLUSH(wx);
+}
+
 /**
  *  txgbe_init_thermal_sensor_thresh - Inits thermal sensor thresholds
  *  @wx: pointer to hardware structure
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h
index e82f65dff8a6..abc729eb187a 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.h
@@ -4,6 +4,8 @@
 #ifndef _TXGBE_HW_H_
 #define _TXGBE_HW_H_
 
+int txgbe_disable_sec_tx_path(struct wx *wx);
+void txgbe_enable_sec_tx_path(struct wx *wx);
 int txgbe_read_pba_string(struct wx *wx, u8 *pba_num, u32 pba_num_size);
 int txgbe_validate_eeprom_checksum(struct wx *wx, u16 *checksum_val);
 int txgbe_reset_hw(struct wx *wx);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 8779645a54be..30a5ed2ed316 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -18,6 +18,7 @@
 #include "../libwx/wx_hw.h"
 #include "txgbe_type.h"
 #include "txgbe_phy.h"
+#include "txgbe_hw.h"
 
 static int txgbe_swnodes_register(struct txgbe *txgbe)
 {
@@ -133,7 +134,7 @@ static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
 	if (!mii_bus)
 		return -ENOMEM;
 
-	mii_bus->name = "txgbe_pcs_mdio_bus";
+	mii_bus->name = DW_MII_BUS_TXGBE;
 	mii_bus->read_c45 = &txgbe_pcs_read;
 	mii_bus->write_c45 = &txgbe_pcs_write;
 	mii_bus->parent = &pdev->dev;
@@ -185,6 +186,8 @@ static void txgbe_mac_link_up(struct phylink_config *config,
 	struct wx *wx = netdev_priv(to_net_dev(config->dev));
 	u32 txcfg, wdg;
 
+	txgbe_enable_sec_tx_path(wx);
+
 	txcfg = rd32(wx, WX_MAC_TX_CFG);
 	txcfg &= ~WX_MAC_TX_CFG_SPEED_MASK;
 
@@ -210,8 +213,20 @@ static void txgbe_mac_link_up(struct phylink_config *config,
 	wr32(wx, WX_MAC_WDG_TIMEOUT, wdg);
 }
 
+static int txgbe_mac_prepare(struct phylink_config *config, unsigned int mode,
+			     phy_interface_t interface)
+{
+	struct wx *wx = netdev_priv(to_net_dev(config->dev));
+
+	wr32m(wx, WX_MAC_TX_CFG, WX_MAC_TX_CFG_TE, 0);
+	wr32m(wx, WX_MAC_RX_CFG, WX_MAC_RX_CFG_RE, 0);
+
+	return txgbe_disable_sec_tx_path(wx);
+}
+
 static const struct phylink_mac_ops txgbe_mac_ops = {
 	.mac_select_pcs = txgbe_phylink_mac_select,
+	.mac_prepare = txgbe_mac_prepare,
 	.mac_config = txgbe_mac_config,
 	.mac_link_down = txgbe_mac_link_down,
 	.mac_link_up = txgbe_mac_link_up,
@@ -234,6 +249,8 @@ static int txgbe_phylink_init(struct txgbe *txgbe)
 	config->mac_capabilities = MAC_10000FD | MAC_1000FD | MAC_SYM_PAUSE | MAC_ASYM_PAUSE;
 	phy_mode = PHY_INTERFACE_MODE_10GBASER;
 	__set_bit(PHY_INTERFACE_MODE_10GBASER, config->supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_1000BASEX, config->supported_interfaces);
+	__set_bit(PHY_INTERFACE_MODE_SGMII, config->supported_interfaces);
 	fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_PHYLINK]);
 	phylink = phylink_create(config, fwnode, phy_mode, &txgbe_mac_ops);
 	if (IS_ERR(phylink))
@@ -431,7 +448,8 @@ static void txgbe_irq_handler(struct irq_desc *desc)
 
 	chained_irq_exit(chip, desc);
 
-	if (eicr & (TXGBE_PX_MISC_ETH_LK | TXGBE_PX_MISC_ETH_LKDN)) {
+	if (eicr & (TXGBE_PX_MISC_ETH_LK | TXGBE_PX_MISC_ETH_LKDN |
+		    TXGBE_PX_MISC_ETH_AN)) {
 		u32 reg = rd32(wx, TXGBE_CFG_PORT_ST);
 
 		phylink_mac_change(txgbe->phylink, !!(reg & TXGBE_CFG_PORT_ST_LINK_UP));
-- 
2.27.0

Re: [PATCH net-next 5/7] net: txgbe: support switching mode to 1000BASE-X and SGMII

[Russell King (Oracle)]

On Mon, Jul 24, 2023 at 06:23:39PM +0800, Jiawen Wu wrote:
> @@ -185,6 +186,8 @@ static void txgbe_mac_link_up(struct phylink_config *config,
>  	struct wx *wx = netdev_priv(to_net_dev(config->dev));
>  	u32 txcfg, wdg;
>  
> +	txgbe_enable_sec_tx_path(wx);
> +
>  	txcfg = rd32(wx, WX_MAC_TX_CFG);
>  	txcfg &= ~WX_MAC_TX_CFG_SPEED_MASK;
>  
> @@ -210,8 +213,20 @@ static void txgbe_mac_link_up(struct phylink_config *config,
>  	wr32(wx, WX_MAC_WDG_TIMEOUT, wdg);
>  }
>  
> +static int txgbe_mac_prepare(struct phylink_config *config, unsigned int mode,
> +			     phy_interface_t interface)
> +{
> +	struct wx *wx = netdev_priv(to_net_dev(config->dev));
> +
> +	wr32m(wx, WX_MAC_TX_CFG, WX_MAC_TX_CFG_TE, 0);
> +	wr32m(wx, WX_MAC_RX_CFG, WX_MAC_RX_CFG_RE, 0);
> +
> +	return txgbe_disable_sec_tx_path(wx);

Is there a reason why the sec_tx_path is enabled/disabled asymmetrically?

I would expect the transmit path to be disabled in mac_link_down() and
re-enabled in mac_link_up().

Alternatively, if it just needs to be disabled for reconfiguration,
I would expect it to be disabled in mac_prepare() and re-enabled in
mac_finish().

The disable in mac_prepare() and enable in mac_link_up() just looks
rather strange, because it isn't symmetrical.

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 5/7] net: txgbe: support switching mode to 1000BASE-X and SGMII

[Jiawen Wu]

On Monday, July 24, 2023 6:40 PM, Russell King (Oracle) wrote:
> On Mon, Jul 24, 2023 at 06:23:39PM +0800, Jiawen Wu wrote:
> > @@ -185,6 +186,8 @@ static void txgbe_mac_link_up(struct phylink_config *config,
> >  	struct wx *wx = netdev_priv(to_net_dev(config->dev));
> >  	u32 txcfg, wdg;
> >
> > +	txgbe_enable_sec_tx_path(wx);
> > +
> >  	txcfg = rd32(wx, WX_MAC_TX_CFG);
> >  	txcfg &= ~WX_MAC_TX_CFG_SPEED_MASK;
> >
> > @@ -210,8 +213,20 @@ static void txgbe_mac_link_up(struct phylink_config *config,
> >  	wr32(wx, WX_MAC_WDG_TIMEOUT, wdg);
> >  }
> >
> > +static int txgbe_mac_prepare(struct phylink_config *config, unsigned int mode,
> > +			     phy_interface_t interface)
> > +{
> > +	struct wx *wx = netdev_priv(to_net_dev(config->dev));
> > +
> > +	wr32m(wx, WX_MAC_TX_CFG, WX_MAC_TX_CFG_TE, 0);
> > +	wr32m(wx, WX_MAC_RX_CFG, WX_MAC_RX_CFG_RE, 0);
> > +
> > +	return txgbe_disable_sec_tx_path(wx);
> 
> Is there a reason why the sec_tx_path is enabled/disabled asymmetrically?
> 
> I would expect the transmit path to be disabled in mac_link_down() and
> re-enabled in mac_link_up().
> 
> Alternatively, if it just needs to be disabled for reconfiguration,
> I would expect it to be disabled in mac_prepare() and re-enabled in
> mac_finish().
> 
> The disable in mac_prepare() and enable in mac_link_up() just looks
> rather strange, because it isn't symmetrical.

It needs to be disabled for PCS switch mode, I will move the re-enable to
mac_finish().

[PATCH net-next 6/7] net: txgbe: support copper NIC with external PHY

[Jiawen Wu]

Wangxun SP chip supports to connect with external PHY (marvell 88x3310),
which links to 10GBASE-T/1000BASE-T/100BASE-T. Add the identification of
media types from subsystem device IDs. For sp_media_copper, register mdio
bus for the external PHY.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/Kconfig          |   1 +
 drivers/net/ethernet/wangxun/libwx/wx_type.h  |  26 +++
 .../ethernet/wangxun/txgbe/txgbe_ethtool.c    |   9 +
 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c |  13 +-
 .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  83 +++++++--
 .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 169 ++++++++++++++++++
 6 files changed, 281 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/Kconfig b/drivers/net/ethernet/wangxun/Kconfig
index 39596cd13539..23cd610bd376 100644
--- a/drivers/net/ethernet/wangxun/Kconfig
+++ b/drivers/net/ethernet/wangxun/Kconfig
@@ -41,6 +41,7 @@ config TXGBE
 	tristate "Wangxun(R) 10GbE PCI Express adapters support"
 	depends on PCI
 	depends on COMMON_CLK
+	select MARVELL_10G_PHY
 	select REGMAP
 	select I2C
 	select I2C_DESIGNWARE_PLATFORM
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
index 50b92cfb46a0..c5cbd177ef62 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
@@ -233,6 +233,24 @@
 #define WX_MAC_WDG_TIMEOUT           0x1100C
 #define WX_MAC_RX_FLOW_CTRL          0x11090
 #define WX_MAC_RX_FLOW_CTRL_RFE      BIT(0) /* receive fc enable */
+/* MDIO Registers */
+#define WX_MSCA                      0x11200
+#define WX_MSCA_RA(v)                FIELD_PREP(U16_MAX, v)
+#define WX_MSCA_PA(v)                FIELD_PREP(GENMASK(20, 16), v)
+#define WX_MSCA_DA(v)                FIELD_PREP(GENMASK(25, 21), v)
+#define WX_MSCC                      0x11204
+#define WX_MSCC_CMD(v)               FIELD_PREP(GENMASK(17, 16), v)
+
+enum WX_MSCA_CMD_value {
+	WX_MSCA_CMD_RSV = 0,
+	WX_MSCA_CMD_WRITE,
+	WX_MSCA_CMD_POST_READ,
+	WX_MSCA_CMD_READ,
+};
+
+#define WX_MSCC_SADDR                BIT(18)
+#define WX_MSCC_BUSY                 BIT(22)
+#define WX_MDIO_CLK(v)               FIELD_PREP(GENMASK(21, 19), v)
 #define WX_MMC_CONTROL               0x11800
 #define WX_MMC_CONTROL_RSTONRD       BIT(2) /* reset on read */
 
@@ -582,6 +600,13 @@ enum wx_mac_type {
 	wx_mac_em
 };
 
+enum sp_media_type {
+	sp_media_unknown = 0,
+	sp_media_fiber,
+	sp_media_copper,
+	sp_media_backplane
+};
+
 enum em_mac_type {
 	em_mac_type_unknown = 0,
 	em_mac_type_mdi,
@@ -829,6 +854,7 @@ struct wx {
 	struct wx_bus_info bus;
 	struct wx_mac_info mac;
 	enum em_mac_type mac_type;
+	enum sp_media_type media_type;
 	struct wx_eeprom_info eeprom;
 	struct wx_addr_filter_info addr_ctrl;
 	struct wx_mac_addr *mac_table;
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
index 859da112586a..889eb8251adc 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
@@ -14,6 +14,9 @@ static int txgbe_nway_reset(struct net_device *netdev)
 {
 	struct txgbe *txgbe = netdev_to_txgbe(netdev);
 
+	if (txgbe->wx->media_type == sp_media_copper)
+		return phy_ethtool_nway_reset(netdev);
+
 	return phylink_ethtool_nway_reset(txgbe->phylink);
 }
 
@@ -22,6 +25,9 @@ static int txgbe_get_link_ksettings(struct net_device *netdev,
 {
 	struct txgbe *txgbe = netdev_to_txgbe(netdev);
 
+	if (txgbe->wx->media_type == sp_media_copper)
+		return phy_ethtool_get_link_ksettings(netdev, cmd);
+
 	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);
 }
 
@@ -30,6 +36,9 @@ static int txgbe_set_link_ksettings(struct net_device *netdev,
 {
 	struct txgbe *txgbe = netdev_to_txgbe(netdev);
 
+	if (txgbe->wx->media_type == sp_media_copper)
+		return phy_ethtool_set_link_ksettings(netdev, cmd);
+
 	return phylink_ethtool_ksettings_set(txgbe->phylink, cmd);
 }
 
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
index 90168aab11ae..372745250270 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
@@ -285,17 +285,20 @@ static void txgbe_reset_misc(struct wx *wx)
 int txgbe_reset_hw(struct wx *wx)
 {
 	int status;
-	u32 val;
 
 	/* Call adapter stop to disable tx/rx and clear interrupts */
 	status = wx_stop_adapter(wx);
 	if (status != 0)
 		return status;
 
-	val = WX_MIS_RST_LAN_RST(wx->bus.func);
-	wr32(wx, WX_MIS_RST, val | rd32(wx, WX_MIS_RST));
-	WX_WRITE_FLUSH(wx);
-	usleep_range(10, 100);
+	if (wx->media_type != sp_media_copper) {
+		u32 val;
+
+		val = WX_MIS_RST_LAN_RST(wx->bus.func);
+		wr32(wx, WX_MIS_RST, val | rd32(wx, WX_MIS_RST));
+		WX_WRITE_FLUSH(wx);
+		usleep_range(10, 100);
+	}
 
 	status = wx_check_flash_load(wx, TXGBE_SPI_ILDR_STATUS_LAN_SW_RST(wx->bus.func));
 	if (status != 0)
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
index 46eba6d6188b..acdbc1f19449 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
@@ -203,10 +203,31 @@ static int txgbe_request_irq(struct wx *wx)
 	return err;
 }
 
+static void txgbe_start_phy(struct wx *wx)
+{
+	if (wx->media_type == sp_media_fiber) {
+		struct txgbe *txgbe = wx->priv;
+
+		phylink_start(txgbe->phylink);
+	} else if (wx->media_type == sp_media_copper) {
+		phy_start(wx->phydev);
+	}
+}
+
+static void txgbe_stop_phy(struct wx *wx)
+{
+	if (wx->media_type == sp_media_fiber) {
+		struct txgbe *txgbe = wx->priv;
+
+		phylink_stop(txgbe->phylink);
+	} else if (wx->media_type == sp_media_copper) {
+		phy_stop(wx->phydev);
+	}
+}
+
 static void txgbe_up_complete(struct wx *wx)
 {
 	struct net_device *netdev = wx->netdev;
-	struct txgbe *txgbe;
 
 	wx_control_hw(wx, true);
 	wx_configure_vectors(wx);
@@ -215,8 +236,7 @@ static void txgbe_up_complete(struct wx *wx)
 	smp_mb__before_atomic();
 	wx_napi_enable_all(wx);
 
-	txgbe = netdev_to_txgbe(netdev);
-	phylink_start(txgbe->phylink);
+	txgbe_start_phy(wx);
 
 	/* clear any pending interrupts, may auto mask */
 	rd32(wx, WX_PX_IC(0));
@@ -290,16 +310,57 @@ static void txgbe_disable_device(struct wx *wx)
 
 static void txgbe_down(struct wx *wx)
 {
-	struct txgbe *txgbe = netdev_to_txgbe(wx->netdev);
-
 	txgbe_disable_device(wx);
 	txgbe_reset(wx);
-	phylink_stop(txgbe->phylink);
+	txgbe_stop_phy(wx);
 
 	wx_clean_all_tx_rings(wx);
 	wx_clean_all_rx_rings(wx);
 }
 
+/**
+ *  txgbe_init_type_code - Initialize the shared code
+ *  @wx: pointer to hardware structure
+ **/
+static void txgbe_init_type_code(struct wx *wx)
+{
+	u8 device_type = wx->subsystem_device_id & 0xF0;
+
+	switch (wx->device_id) {
+	case TXGBE_DEV_ID_SP1000:
+	case TXGBE_DEV_ID_WX1820:
+		wx->mac.type = wx_mac_sp;
+		break;
+	default:
+		wx->mac.type = wx_mac_unknown;
+		break;
+	}
+
+	switch (device_type) {
+	case TXGBE_ID_SFP:
+		wx->media_type = sp_media_fiber;
+		break;
+	case TXGBE_ID_XAUI:
+	case TXGBE_ID_SGMII:
+		wx->media_type = sp_media_copper;
+		break;
+	case TXGBE_ID_KR_KX_KX4:
+	case TXGBE_ID_MAC_XAUI:
+	case TXGBE_ID_MAC_SGMII:
+		wx->media_type = sp_media_backplane;
+		break;
+	case TXGBE_ID_SFI_XAUI:
+		if (wx->bus.func == 0)
+			wx->media_type = sp_media_fiber;
+		else
+			wx->media_type = sp_media_copper;
+		break;
+	default:
+		wx->media_type = sp_media_unknown;
+		break;
+	}
+}
+
 /**
  * txgbe_sw_init - Initialize general software structures (struct wx)
  * @wx: board private structure to initialize
@@ -324,15 +385,7 @@ static int txgbe_sw_init(struct wx *wx)
 		return err;
 	}
 
-	switch (wx->device_id) {
-	case TXGBE_DEV_ID_SP1000:
-	case TXGBE_DEV_ID_WX1820:
-		wx->mac.type = wx_mac_sp;
-		break;
-	default:
-		wx->mac.type = wx_mac_unknown;
-		break;
-	}
+	txgbe_init_type_code(wx);
 
 	/* Set common capability flags and settings */
 	wx->max_q_vectors = TXGBE_MAX_MSIX_VECTORS;
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
index 30a5ed2ed316..233b1b0fa274 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
@@ -616,10 +616,174 @@ static int txgbe_sfp_register(struct txgbe *txgbe)
 	return 0;
 }
 
+static int txgbe_phy_read(struct mii_bus *bus, int phy_addr,
+			  int devnum, int regnum)
+{
+	struct wx *wx = bus->priv;
+	u32 val, command;
+	int ret;
+
+	/* setup and write the address cycle command */
+	command = WX_MSCA_RA(regnum) |
+		  WX_MSCA_PA(phy_addr) |
+		  WX_MSCA_DA(devnum);
+	wr32(wx, WX_MSCA, command);
+
+	command = WX_MSCC_CMD(WX_MSCA_CMD_READ) | WX_MSCC_BUSY;
+	wr32(wx, WX_MSCC, command);
+
+	/* wait to complete */
+	ret = read_poll_timeout(rd32, val, !(val & WX_MSCC_BUSY), 1000,
+				100000, false, wx, WX_MSCC);
+	if (ret) {
+		wx_err(wx, "Mdio read c45 command did not complete.\n");
+		return ret;
+	}
+
+	return (u16)rd32(wx, WX_MSCC);
+}
+
+static int txgbe_phy_write(struct mii_bus *bus, int phy_addr,
+			   int devnum, int regnum, u16 value)
+{
+	struct wx *wx = bus->priv;
+	int ret, command;
+	u16 val;
+
+	/* setup and write the address cycle command */
+	command = WX_MSCA_RA(regnum) |
+		  WX_MSCA_PA(phy_addr) |
+		  WX_MSCA_DA(devnum);
+	wr32(wx, WX_MSCA, command);
+
+	command = value | WX_MSCC_CMD(WX_MSCA_CMD_WRITE) | WX_MSCC_BUSY;
+	wr32(wx, WX_MSCC, command);
+
+	/* wait to complete */
+	ret = read_poll_timeout(rd32, val, !(val & WX_MSCC_BUSY), 1000,
+				100000, false, wx, WX_MSCC);
+	if (ret)
+		wx_err(wx, "Mdio write c45 command did not complete.\n");
+
+	return ret;
+}
+
+static void txgbe_handle_phy_link(struct net_device *dev)
+{
+	struct wx *wx = netdev_priv(dev);
+	struct phy_device *phydev;
+	u32 txcfg, wdg;
+
+	phydev = wx->phydev;
+	if (!(wx->link != phydev->link ||
+	      wx->speed != phydev->speed ||
+	      wx->duplex != phydev->duplex))
+		return;
+
+	wx->link = phydev->link;
+	wx->speed = phydev->speed;
+	wx->duplex = phydev->duplex;
+
+	if (!phydev->link)
+		goto out;
+
+	txcfg = rd32(wx, WX_MAC_TX_CFG);
+	txcfg &= ~WX_MAC_TX_CFG_SPEED_MASK;
+
+	switch (phydev->speed) {
+	case SPEED_10000:
+		txcfg |= WX_MAC_TX_CFG_SPEED_10G;
+		break;
+	case SPEED_1000:
+	case SPEED_100:
+	case SPEED_10:
+		txcfg |= WX_MAC_TX_CFG_SPEED_1G;
+		break;
+	default:
+		break;
+	}
+
+	wr32(wx, WX_MAC_TX_CFG, txcfg | WX_MAC_TX_CFG_TE);
+
+	/* Re configure MAC Rx */
+	wr32m(wx, WX_MAC_RX_CFG, WX_MAC_RX_CFG_RE, WX_MAC_RX_CFG_RE);
+	wr32(wx, WX_MAC_PKT_FLT, WX_MAC_PKT_FLT_PR);
+	wdg = rd32(wx, WX_MAC_WDG_TIMEOUT);
+	wr32(wx, WX_MAC_WDG_TIMEOUT, wdg);
+
+out:
+	phy_print_status(phydev);
+}
+
+static int txgbe_mdio_phy_init(struct txgbe *txgbe)
+{
+	struct phy_device *phydev;
+	struct mii_bus *mii_bus;
+	struct pci_dev *pdev;
+	struct wx *wx;
+	int ret = 0;
+
+	wx = txgbe->wx;
+	pdev = wx->pdev;
+
+	mii_bus = devm_mdiobus_alloc(&pdev->dev);
+	if (!mii_bus)
+		return -ENOMEM;
+
+	mii_bus->name = "txgbe_mii_bus";
+	mii_bus->read_c45 = &txgbe_phy_read;
+	mii_bus->write_c45 = &txgbe_phy_write;
+	mii_bus->parent = &pdev->dev;
+	mii_bus->phy_mask = GENMASK(31, 1);
+	mii_bus->priv = wx;
+	snprintf(mii_bus->id, MII_BUS_ID_SIZE, "txgbe-%x",
+		 (pdev->bus->number << 8) | pdev->devfn);
+
+	ret = devm_mdiobus_register(&pdev->dev, mii_bus);
+	if (ret) {
+		wx_err(wx, "failed to register MDIO bus: %d\n", ret);
+		return ret;
+	}
+
+	phydev = phy_find_first(mii_bus);
+	if (!phydev) {
+		wx_err(wx, "no PHY found\n");
+		return -ENODEV;
+	}
+
+	phy_attached_info(phydev);
+
+	/* remove unsupport link mode */
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_2500baseT_Full_BIT);
+	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_5000baseT_Full_BIT);
+
+	wx->link = 0;
+	wx->speed = 0;
+	wx->duplex = 0;
+	wx->phydev = phydev;
+
+	ret = phy_connect_direct(wx->netdev, phydev,
+				 txgbe_handle_phy_link,
+				 PHY_INTERFACE_MODE_XAUI);
+	if (ret) {
+		wx_err(wx, "PHY connect failed: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 int txgbe_init_phy(struct txgbe *txgbe)
 {
 	int ret;
 
+	if (txgbe->wx->media_type == sp_media_copper)
+		return txgbe_mdio_phy_init(txgbe);
+
 	ret = txgbe_swnodes_register(txgbe);
 	if (ret) {
 		wx_err(txgbe->wx, "failed to register software nodes\n");
@@ -681,6 +845,11 @@ int txgbe_init_phy(struct txgbe *txgbe)
 
 void txgbe_remove_phy(struct txgbe *txgbe)
 {
+	if (txgbe->wx->media_type == sp_media_copper) {
+		phy_disconnect(txgbe->wx->phydev);
+		return;
+	}
+
 	platform_device_unregister(txgbe->sfp_dev);
 	platform_device_unregister(txgbe->i2c_dev);
 	clkdev_drop(txgbe->clock);
-- 
2.27.0

Re: [PATCH net-next 6/7] net: txgbe: support copper NIC with external PHY

[Russell King (Oracle)]

On Mon, Jul 24, 2023 at 06:23:40PM +0800, Jiawen Wu wrote:
> @@ -22,6 +25,9 @@ static int txgbe_get_link_ksettings(struct net_device *netdev,
>  {
>  	struct txgbe *txgbe = netdev_to_txgbe(netdev);
>  
> +	if (txgbe->wx->media_type == sp_media_copper)
> +		return phy_ethtool_get_link_ksettings(netdev, cmd);

Why? If a PHY is attached via phylink, then phylink will automatically
forward the call below to phylib.
> +
>  	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);

If you implement it correctly, you also don't need two entirely
separate paths to configure the MAC/PCS for the results of the PHY's
negotiation, because phylink gives you a _generic_ set of interfaces
between whatever is downstream from the MAC and the MAC.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 6/7] net: txgbe: support copper NIC with external PHY

[Jiawen Wu]

On Monday, July 24, 2023 6:43 PM, Russell King (Oracle) wrote:
> On Mon, Jul 24, 2023 at 06:23:40PM +0800, Jiawen Wu wrote:
> > @@ -22,6 +25,9 @@ static int txgbe_get_link_ksettings(struct net_device *netdev,
> >  {
> >  	struct txgbe *txgbe = netdev_to_txgbe(netdev);
> >
> > +	if (txgbe->wx->media_type == sp_media_copper)
> > +		return phy_ethtool_get_link_ksettings(netdev, cmd);
> 
> Why? If a PHY is attached via phylink, then phylink will automatically
> forward the call below to phylib.

No, there is no phylink implemented for sp_media_copper.

> > +
> >  	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);
> 
> If you implement it correctly, you also don't need two entirely
> separate paths to configure the MAC/PCS for the results of the PHY's
> negotiation, because phylink gives you a _generic_ set of interfaces
> between whatever is downstream from the MAC and the MAC.

For sp_media_copper, only mii bus is registered for attaching PHY.
Most MAC/PCS configuration is done in firmware, so it is not necessary
to implement phylink as sp_media_fiber.

Re: [PATCH net-next 6/7] net: txgbe: support copper NIC with external PHY

[Russell King (Oracle)]

On Tue, Jul 25, 2023 at 10:41:46AM +0800, Jiawen Wu wrote:
> On Monday, July 24, 2023 6:43 PM, Russell King (Oracle) wrote:
> > On Mon, Jul 24, 2023 at 06:23:40PM +0800, Jiawen Wu wrote:
> > > @@ -22,6 +25,9 @@ static int txgbe_get_link_ksettings(struct net_device *netdev,
> > >  {
> > >  	struct txgbe *txgbe = netdev_to_txgbe(netdev);
> > >
> > > +	if (txgbe->wx->media_type == sp_media_copper)
> > > +		return phy_ethtool_get_link_ksettings(netdev, cmd);
> > 
> > Why? If a PHY is attached via phylink, then phylink will automatically
> > forward the call below to phylib.
> 
> No, there is no phylink implemented for sp_media_copper.
> 
> > > +
> > >  	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);
> > 
> > If you implement it correctly, you also don't need two entirely
> > separate paths to configure the MAC/PCS for the results of the PHY's
> > negotiation, because phylink gives you a _generic_ set of interfaces
> > between whatever is downstream from the MAC and the MAC.
> 
> For sp_media_copper, only mii bus is registered for attaching PHY.
> Most MAC/PCS configuration is done in firmware, so it is not necessary
> to implement phylink as sp_media_fiber.

If you do implement phylink for copper, then you don't need all these
conditionals and the additional adjust_link implementation. In other
words, you can re-use a lot of the code you've already added.

You don't have to provide a PCS to phylink provided you don't tell
phylink that it's "in-band".

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH net-next 6/7] net: txgbe: support copper NIC with external PHY

[Jiawen Wu]

On Tuesday, July 25, 2023 4:03 PM, Russell King (Oracle) wrote:
> On Tue, Jul 25, 2023 at 10:41:46AM +0800, Jiawen Wu wrote:
> > On Monday, July 24, 2023 6:43 PM, Russell King (Oracle) wrote:
> > > On Mon, Jul 24, 2023 at 06:23:40PM +0800, Jiawen Wu wrote:
> > > > @@ -22,6 +25,9 @@ static int txgbe_get_link_ksettings(struct net_device *netdev,
> > > >  {
> > > >  	struct txgbe *txgbe = netdev_to_txgbe(netdev);
> > > >
> > > > +	if (txgbe->wx->media_type == sp_media_copper)
> > > > +		return phy_ethtool_get_link_ksettings(netdev, cmd);
> > >
> > > Why? If a PHY is attached via phylink, then phylink will automatically
> > > forward the call below to phylib.
> >
> > No, there is no phylink implemented for sp_media_copper.
> >
> > > > +
> > > >  	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);
> > >
> > > If you implement it correctly, you also don't need two entirely
> > > separate paths to configure the MAC/PCS for the results of the PHY's
> > > negotiation, because phylink gives you a _generic_ set of interfaces
> > > between whatever is downstream from the MAC and the MAC.
> >
> > For sp_media_copper, only mii bus is registered for attaching PHY.
> > Most MAC/PCS configuration is done in firmware, so it is not necessary
> > to implement phylink as sp_media_fiber.
> 
> If you do implement phylink for copper, then you don't need all these
> conditionals and the additional adjust_link implementation. In other
> words, you can re-use a lot of the code you've already added.
> 
> You don't have to provide a PCS to phylink provided you don't tell
> phylink that it's "in-band".

Do I need to create a separate software node? That would seem to
break more code of fiber initialization flow. I could try, but I'd like to
keep the two flows separate.

Re: [PATCH net-next 6/7] net: txgbe: support copper NIC with external PHY

[Russell King (Oracle)]

On Tue, Jul 25, 2023 at 06:04:49PM +0800, Jiawen Wu wrote:
> On Tuesday, July 25, 2023 4:03 PM, Russell King (Oracle) wrote:
> > On Tue, Jul 25, 2023 at 10:41:46AM +0800, Jiawen Wu wrote:
> > > On Monday, July 24, 2023 6:43 PM, Russell King (Oracle) wrote:
> > > > On Mon, Jul 24, 2023 at 06:23:40PM +0800, Jiawen Wu wrote:
> > > > > @@ -22,6 +25,9 @@ static int txgbe_get_link_ksettings(struct net_device *netdev,
> > > > >  {
> > > > >  	struct txgbe *txgbe = netdev_to_txgbe(netdev);
> > > > >
> > > > > +	if (txgbe->wx->media_type == sp_media_copper)
> > > > > +		return phy_ethtool_get_link_ksettings(netdev, cmd);
> > > >
> > > > Why? If a PHY is attached via phylink, then phylink will automatically
> > > > forward the call below to phylib.
> > >
> > > No, there is no phylink implemented for sp_media_copper.
> > >
> > > > > +
> > > > >  	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);
> > > >
> > > > If you implement it correctly, you also don't need two entirely
> > > > separate paths to configure the MAC/PCS for the results of the PHY's
> > > > negotiation, because phylink gives you a _generic_ set of interfaces
> > > > between whatever is downstream from the MAC and the MAC.
> > >
> > > For sp_media_copper, only mii bus is registered for attaching PHY.
> > > Most MAC/PCS configuration is done in firmware, so it is not necessary
> > > to implement phylink as sp_media_fiber.
> > 
> > If you do implement phylink for copper, then you don't need all these
> > conditionals and the additional adjust_link implementation. In other
> > words, you can re-use a lot of the code you've already added.
> > 
> > You don't have to provide a PCS to phylink provided you don't tell
> > phylink that it's "in-band".
> 
> Do I need to create a separate software node? That would seem to
> break more code of fiber initialization flow. I could try, but I'd like to
> keep the two flows separate.

You don't need any of the swnodes to be registered, so
txgbe_swnodes_register() can be skipped. You also don't need
txgbe_mdio_pcs_init() as you said firmware will look after that.

You will need txgbe_phylink_init() to select phy_mode depending on
whether your configuration is for SFP or not, so something like:

	if (txgbe->wx->media_type == sp_media_copper) {
		phy_mode = PHY_INTERFACE_MODE_XAUI;
		fwnode = NULL;
	} else {
		phy_mode = PHY_INTERFACE_MODE_10GBASER;
		fwnode = software_node_fwnode(txgbe->nodes.group[SWNODE_PHYLINK]);
	}

	__set_bit(phy_mode, config->supported_interfaces);
	phylink = phylink_create(config, fwnode, phy_mode, &txgbe_mac_ops);

You can then use phylink_connect_phy() to add the phydev to phylink.

You'll probably need to make txgbe_phylink_mac_select() check whether
txgbe->xpcs is non-NULL to prevent a NULL pointer dereference as I
don't believe you have the XPCS in this setup - or alternatively:

	if (interface == PHY_INTERFACE_MODE_10GBASER)
		return &txgbe->xpcs->pcs;

	return NULL;

and that should be about all that would be required. phylink will
then forward all the appropriate calls onto phylib for you, take care
of reading the phy's status, and calling the mac_link_up/mac_link_down
functions as the PHY status indicates the link changes state.

Phylink will call mac_prepare()/mac_config()/mac_finish() when the
netdev is opened, and will also limit the PHY's advertisement
according to the capabilities supplied in mac_capabilities, so you
shouldn't need to remove unsupported link modes from the PHY.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next 6/7] net: txgbe: support copper NIC with external PHY

[Simon Horman]

On Mon, Jul 24, 2023 at 06:23:40PM +0800, Jiawen Wu wrote:

...

Hi Jiawen Wu,

a minor nit from my side.

> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c

...

> +
> +	/* remove unsupport link mode */

nit: unsupport -> unsupported

> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_100baseT_Half_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_2500baseT_Full_BIT);
> +	phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_5000baseT_Full_BIT);

...

[PATCH net-next 7/7] net: ngbe: move mdio access registers to libwx

[Jiawen Wu]

Registers of mdio accessing are common defined in libwx, remove the
redundant macro definitions in ngbe driver.

Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c | 84 +++++++++----------
 drivers/net/ethernet/wangxun/ngbe/ngbe_type.h | 19 -----
 2 files changed, 42 insertions(+), 61 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c
index cc2f325a52f7..60fc996bb53e 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_mdio.c
@@ -37,24 +37,24 @@ static int ngbe_phy_read_reg_mdi_c22(struct mii_bus *bus, int phy_addr, int regn
 
 	wr32(wx, NGBE_MDIO_CLAUSE_SELECT, 0xF);
 	/* setup and write the address cycle command */
-	command = NGBE_MSCA_RA(regnum) |
-		  NGBE_MSCA_PA(phy_addr) |
-		  NGBE_MSCA_DA(device_type);
-	wr32(wx, NGBE_MSCA, command);
-	command = NGBE_MSCC_CMD(NGBE_MSCA_CMD_READ) |
-		  NGBE_MSCC_BUSY |
-		  NGBE_MDIO_CLK(6);
-	wr32(wx, NGBE_MSCC, command);
+	command = WX_MSCA_RA(regnum) |
+		  WX_MSCA_PA(phy_addr) |
+		  WX_MSCA_DA(device_type);
+	wr32(wx, WX_MSCA, command);
+	command = WX_MSCC_CMD(WX_MSCA_CMD_READ) |
+		  WX_MSCC_BUSY |
+		  WX_MDIO_CLK(6);
+	wr32(wx, WX_MSCC, command);
 
 	/* wait to complete */
-	ret = read_poll_timeout(rd32, val, !(val & NGBE_MSCC_BUSY), 1000,
-				100000, false, wx, NGBE_MSCC);
+	ret = read_poll_timeout(rd32, val, !(val & WX_MSCC_BUSY), 1000,
+				100000, false, wx, WX_MSCC);
 	if (ret) {
 		wx_err(wx, "Mdio read c22 command did not complete.\n");
 		return ret;
 	}
 
-	return (u16)rd32(wx, NGBE_MSCC);
+	return (u16)rd32(wx, WX_MSCC);
 }
 
 static int ngbe_phy_write_reg_mdi_c22(struct mii_bus *bus, int phy_addr, int regnum, u16 value)
@@ -65,19 +65,19 @@ static int ngbe_phy_write_reg_mdi_c22(struct mii_bus *bus, int phy_addr, int reg
 
 	wr32(wx, NGBE_MDIO_CLAUSE_SELECT, 0xF);
 	/* setup and write the address cycle command */
-	command = NGBE_MSCA_RA(regnum) |
-		  NGBE_MSCA_PA(phy_addr) |
-		  NGBE_MSCA_DA(device_type);
-	wr32(wx, NGBE_MSCA, command);
+	command = WX_MSCA_RA(regnum) |
+		  WX_MSCA_PA(phy_addr) |
+		  WX_MSCA_DA(device_type);
+	wr32(wx, WX_MSCA, command);
 	command = value |
-		  NGBE_MSCC_CMD(NGBE_MSCA_CMD_WRITE) |
-		  NGBE_MSCC_BUSY |
-		  NGBE_MDIO_CLK(6);
-	wr32(wx, NGBE_MSCC, command);
+		  WX_MSCC_CMD(WX_MSCA_CMD_WRITE) |
+		  WX_MSCC_BUSY |
+		  WX_MDIO_CLK(6);
+	wr32(wx, WX_MSCC, command);
 
 	/* wait to complete */
-	ret = read_poll_timeout(rd32, val, !(val & NGBE_MSCC_BUSY), 1000,
-				100000, false, wx, NGBE_MSCC);
+	ret = read_poll_timeout(rd32, val, !(val & WX_MSCC_BUSY), 1000,
+				100000, false, wx, WX_MSCC);
 	if (ret)
 		wx_err(wx, "Mdio write c22 command did not complete.\n");
 
@@ -92,24 +92,24 @@ static int ngbe_phy_read_reg_mdi_c45(struct mii_bus *bus, int phy_addr, int devn
 
 	wr32(wx, NGBE_MDIO_CLAUSE_SELECT, 0x0);
 	/* setup and write the address cycle command */
-	command = NGBE_MSCA_RA(regnum) |
-		  NGBE_MSCA_PA(phy_addr) |
-		  NGBE_MSCA_DA(devnum);
-	wr32(wx, NGBE_MSCA, command);
-	command = NGBE_MSCC_CMD(NGBE_MSCA_CMD_READ) |
-		  NGBE_MSCC_BUSY |
-		  NGBE_MDIO_CLK(6);
-	wr32(wx, NGBE_MSCC, command);
+	command = WX_MSCA_RA(regnum) |
+		  WX_MSCA_PA(phy_addr) |
+		  WX_MSCA_DA(devnum);
+	wr32(wx, WX_MSCA, command);
+	command = WX_MSCC_CMD(WX_MSCA_CMD_READ) |
+		  WX_MSCC_BUSY |
+		  WX_MDIO_CLK(6);
+	wr32(wx, WX_MSCC, command);
 
 	/* wait to complete */
-	ret = read_poll_timeout(rd32, val, !(val & NGBE_MSCC_BUSY), 1000,
-				100000, false, wx, NGBE_MSCC);
+	ret = read_poll_timeout(rd32, val, !(val & WX_MSCC_BUSY), 1000,
+				100000, false, wx, WX_MSCC);
 	if (ret) {
 		wx_err(wx, "Mdio read c45 command did not complete.\n");
 		return ret;
 	}
 
-	return (u16)rd32(wx, NGBE_MSCC);
+	return (u16)rd32(wx, WX_MSCC);
 }
 
 static int ngbe_phy_write_reg_mdi_c45(struct mii_bus *bus, int phy_addr,
@@ -121,19 +121,19 @@ static int ngbe_phy_write_reg_mdi_c45(struct mii_bus *bus, int phy_addr,
 
 	wr32(wx, NGBE_MDIO_CLAUSE_SELECT, 0x0);
 	/* setup and write the address cycle command */
-	command = NGBE_MSCA_RA(regnum) |
-		  NGBE_MSCA_PA(phy_addr) |
-		  NGBE_MSCA_DA(devnum);
-	wr32(wx, NGBE_MSCA, command);
+	command = WX_MSCA_RA(regnum) |
+		  WX_MSCA_PA(phy_addr) |
+		  WX_MSCA_DA(devnum);
+	wr32(wx, WX_MSCA, command);
 	command = value |
-		  NGBE_MSCC_CMD(NGBE_MSCA_CMD_WRITE) |
-		  NGBE_MSCC_BUSY |
-		  NGBE_MDIO_CLK(6);
-	wr32(wx, NGBE_MSCC, command);
+		  WX_MSCC_CMD(WX_MSCA_CMD_WRITE) |
+		  WX_MSCC_BUSY |
+		  WX_MDIO_CLK(6);
+	wr32(wx, WX_MSCC, command);
 
 	/* wait to complete */
-	ret = read_poll_timeout(rd32, val, !(val & NGBE_MSCC_BUSY), 1000,
-				100000, false, wx, NGBE_MSCC);
+	ret = read_poll_timeout(rd32, val, !(val & WX_MSCC_BUSY), 1000,
+				100000, false, wx, WX_MSCC);
 	if (ret)
 		wx_err(wx, "Mdio write c45 command did not complete.\n");
 
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
index b70eca397b67..72c8cd2d5575 100644
--- a/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
+++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_type.h
@@ -59,25 +59,6 @@
 #define NGBE_EEPROM_VERSION_L			0x1D
 #define NGBE_EEPROM_VERSION_H			0x1E
 
-/* mdio access */
-#define NGBE_MSCA				0x11200
-#define NGBE_MSCA_RA(v)				FIELD_PREP(U16_MAX, v)
-#define NGBE_MSCA_PA(v)				FIELD_PREP(GENMASK(20, 16), v)
-#define NGBE_MSCA_DA(v)				FIELD_PREP(GENMASK(25, 21), v)
-#define NGBE_MSCC				0x11204
-#define NGBE_MSCC_CMD(v)			FIELD_PREP(GENMASK(17, 16), v)
-
-enum NGBE_MSCA_CMD_value {
-	NGBE_MSCA_CMD_RSV = 0,
-	NGBE_MSCA_CMD_WRITE,
-	NGBE_MSCA_CMD_POST_READ,
-	NGBE_MSCA_CMD_READ,
-};
-
-#define NGBE_MSCC_SADDR				BIT(18)
-#define NGBE_MSCC_BUSY				BIT(22)
-#define NGBE_MDIO_CLK(v)			FIELD_PREP(GENMASK(21, 19), v)
-
 /* Media-dependent registers. */
 #define NGBE_MDIO_CLAUSE_SELECT			0x11220
 
-- 
2.27.0