[PATCH net-next v9 0/8] Support loopback mode speed selection

[PATCH net-next v9 0/8] Support loopback mode speed selection

[Gerhard Engleder]

Previously to commit 6ff3cddc365b ("net: phylib: do not disable autoneg
for fixed speeds >= 1G") it was possible to select the speed of the
loopback mode by configuring a fixed speed before enabling the loopback
mode. Now autoneg is always enabled for >= 1G and a fixed speed of >= 1G
requires successful autoneg. Thus, the speed of the loopback mode depends
on the link partner for >= 1G. There is no technical reason to depend on
the link partner for loopback mode. With this behavior the loopback mode
is less useful for testing.

Allow PHYs to support optional speed selection for the loopback mode.
This support is implemented for the generic loopback support and for PHY
drivers, which obviously support speed selection for loopback mode.
Additionally, loopback support according to the data sheet is added to
the KSZ9031 PHY.

Extend phy_loopback() to signal link up and down if speed changes,
because a new link speed requires link up signalling.

Use this loopback speed selection in the tsnep driver to select the
loopback mode speed depending the previously active speed. User space
tests with 100 Mbps and 1 Gbps loopback are possible again.

v9:
- move struct net_test member loopback_speed to correct commit
- fix argument of net_selftest_set_get_count() dummy (kernel test robot)
- fix argument of net_selftest_set_get_strings() dummy (kernel test robot)

v8:
- align naming and kdoc of selftest set enum (Jakub Kicinski)
- add support for all link speeds (Jakub Kicinski)

v7:
- simplify ksz9031_set_loopback() (Andrew Lunn)
- try to restore link if loopback enable fails (Andrew Lunn)
- extend generic selftests to enable reuse (Andrew Lunn)

v6:
- add return value documentation to phy_loopback() (Jakub Kicinski)

v5:
- use phy_write() instead of phy_modify() (Russel King)
- add missing inline for export dummies (kernel test robot)

v4:
- resend without changed to RFC v3

RFC v3:
- align set_loopback() of Marvell to Micrel (Andrew Lunn)
- transmit packets in loopback selftests (Andrew Lunn)
- don't flush PHY statemachine in phy_loopback()
- remove setting of carrier on and link mode after phy_loopback() in tsnep

v2:
- signal link up to keep MAC and PHY in sync about speed (Andrew Lunn)

Gerhard Engleder (8):
  net: phy: Allow loopback speed selection for PHY drivers
  net: phy: Support speed selection for PHY loopback
  net: phy: micrel: Add loopback support
  net: phy: marvell: Align set_loopback() implementation
  tsnep: Select speed for loopback
  net: selftests: Support selftest sets
  net: selftests: Add selftests sets with fixed speed
  tsnep: Add loopback selftests

 drivers/net/ethernet/engleder/tsnep_main.c    |  21 +-
 .../net/ethernet/engleder/tsnep_selftests.c   |  30 ++-
 .../net/ethernet/hisilicon/hns/hns_ethtool.c  |   4 +-
 .../hisilicon/hns3/hns3pf/hclge_main.c        |   4 +-
 .../hisilicon/hns3/hns3pf/hclge_mdio.c        |   2 +-
 .../stmicro/stmmac/stmmac_selftests.c         |   8 +-
 drivers/net/phy/adin1100.c                    |   5 +-
 drivers/net/phy/dp83867.c                     |   5 +-
 drivers/net/phy/marvell.c                     |  68 +++---
 drivers/net/phy/micrel.c                      |  24 ++
 drivers/net/phy/mxl-gpy.c                     |  11 +-
 drivers/net/phy/phy-c45.c                     |   5 +-
 drivers/net/phy/phy.c                         |  87 +++++++
 drivers/net/phy/phy_device.c                  |  43 +---
 drivers/net/phy/xilinx_gmii2rgmii.c           |   7 +-
 include/linux/phy.h                           |  18 +-
 include/net/selftests.h                       |  29 +++
 net/core/selftests.c                          | 216 +++++++++++++-----
 18 files changed, 417 insertions(+), 170 deletions(-)

-- 
2.39.5

[PATCH net-next v9 1/8] net: phy: Allow loopback speed selection for PHY drivers

[Gerhard Engleder]

PHY drivers support loopback mode, but it is not possible to select the
speed of the loopback mode. The speed is chosen by the set_loopback()
operation of the PHY driver. Same is valid for genphy_loopback().

There are PHYs that support loopback with different speeds. Extend
set_loopback() to make loopback speed selection possible.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/adin1100.c          |  5 ++++-
 drivers/net/phy/dp83867.c           |  5 ++++-
 drivers/net/phy/marvell.c           |  8 +++++++-
 drivers/net/phy/mxl-gpy.c           | 11 +++++++----
 drivers/net/phy/phy-c45.c           |  5 ++++-
 drivers/net/phy/phy_device.c        | 12 +++++++++---
 drivers/net/phy/xilinx_gmii2rgmii.c |  7 ++++---
 include/linux/phy.h                 | 16 ++++++++++++----
 8 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/adin1100.c b/drivers/net/phy/adin1100.c
index 6bb469429b9d..bd7a47a903ac 100644
--- a/drivers/net/phy/adin1100.c
+++ b/drivers/net/phy/adin1100.c
@@ -215,8 +215,11 @@ static int adin_resume(struct phy_device *phydev)
 	return adin_set_powerdown_mode(phydev, false);
 }
 
-static int adin_set_loopback(struct phy_device *phydev, bool enable)
+static int adin_set_loopback(struct phy_device *phydev, bool enable, int speed)
 {
+	if (enable && speed)
+		return -EOPNOTSUPP;
+
 	if (enable)
 		return phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MDIO_PCS_10T1L_CTRL,
 					BMCR_LOOPBACK);
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index c1451df430ac..063266cafe9c 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -1009,8 +1009,11 @@ static void dp83867_link_change_notify(struct phy_device *phydev)
 	}
 }
 
-static int dp83867_loopback(struct phy_device *phydev, bool enable)
+static int dp83867_loopback(struct phy_device *phydev, bool enable, int speed)
 {
+	if (enable && speed)
+		return -EOPNOTSUPP;
+
 	return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
 			  enable ? BMCR_LOOPBACK : 0);
 }
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index dd254e36ca8a..f2ad675537d1 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -2131,13 +2131,19 @@ static void marvell_get_stats_simple(struct phy_device *phydev,
 		data[i] = marvell_get_stat_simple(phydev, i);
 }
 
-static int m88e1510_loopback(struct phy_device *phydev, bool enable)
+static int m88e1510_loopback(struct phy_device *phydev, bool enable, int speed)
 {
 	int err;
 
 	if (enable) {
 		u16 bmcr_ctl, mscr2_ctl = 0;
 
+		if (speed == SPEED_10 || speed == SPEED_100 ||
+		    speed == SPEED_1000)
+			phydev->speed = speed;
+		else if (speed)
+			return -EINVAL;
+
 		bmcr_ctl = mii_bmcr_encode_fixed(phydev->speed, phydev->duplex);
 
 		err = phy_write(phydev, MII_BMCR, bmcr_ctl);
diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c
index 94d9cb727121..a6cca8d43253 100644
--- a/drivers/net/phy/mxl-gpy.c
+++ b/drivers/net/phy/mxl-gpy.c
@@ -813,7 +813,7 @@ static void gpy_get_wol(struct phy_device *phydev,
 	wol->wolopts = priv->wolopts;
 }
 
-static int gpy_loopback(struct phy_device *phydev, bool enable)
+static int gpy_loopback(struct phy_device *phydev, bool enable, int speed)
 {
 	struct gpy_priv *priv = phydev->priv;
 	u16 set = 0;
@@ -822,6 +822,9 @@ static int gpy_loopback(struct phy_device *phydev, bool enable)
 	if (enable) {
 		u64 now = get_jiffies_64();
 
+		if (speed)
+			return -EOPNOTSUPP;
+
 		/* wait until 3 seconds from last disable */
 		if (time_before64(now, priv->lb_dis_to))
 			msleep(jiffies64_to_msecs(priv->lb_dis_to - now));
@@ -845,15 +848,15 @@ static int gpy_loopback(struct phy_device *phydev, bool enable)
 	return 0;
 }
 
-static int gpy115_loopback(struct phy_device *phydev, bool enable)
+static int gpy115_loopback(struct phy_device *phydev, bool enable, int speed)
 {
 	struct gpy_priv *priv = phydev->priv;
 
 	if (enable)
-		return gpy_loopback(phydev, enable);
+		return gpy_loopback(phydev, enable, speed);
 
 	if (priv->fw_minor > 0x76)
-		return gpy_loopback(phydev, 0);
+		return gpy_loopback(phydev, 0, 0);
 
 	return genphy_soft_reset(phydev);
 }
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index 0bcbdce38107..8a0ffc3174ec 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -1228,8 +1228,11 @@ int gen10g_config_aneg(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(gen10g_config_aneg);
 
-int genphy_c45_loopback(struct phy_device *phydev, bool enable)
+int genphy_c45_loopback(struct phy_device *phydev, bool enable, int speed)
 {
+	if (enable && speed)
+		return -EOPNOTSUPP;
+
 	return phy_modify_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1,
 			      MDIO_PCS_CTRL1_LOOPBACK,
 			      enable ? MDIO_PCS_CTRL1_LOOPBACK : 0);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index a38d399f244b..77c39f7ed340 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2074,9 +2074,9 @@ int phy_loopback(struct phy_device *phydev, bool enable)
 	}
 
 	if (phydev->drv->set_loopback)
-		ret = phydev->drv->set_loopback(phydev, enable);
+		ret = phydev->drv->set_loopback(phydev, enable, 0);
 	else
-		ret = genphy_loopback(phydev, enable);
+		ret = genphy_loopback(phydev, enable, 0);
 
 	if (ret)
 		goto out;
@@ -2845,12 +2845,18 @@ int genphy_resume(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(genphy_resume);
 
-int genphy_loopback(struct phy_device *phydev, bool enable)
+int genphy_loopback(struct phy_device *phydev, bool enable, int speed)
 {
 	if (enable) {
 		u16 ctl = BMCR_LOOPBACK;
 		int ret, val;
 
+		if (speed == SPEED_10 || speed == SPEED_100 ||
+		    speed == SPEED_1000)
+			phydev->speed = speed;
+		else if (speed)
+			return -EINVAL;
+
 		ctl |= mii_bmcr_encode_fixed(phydev->speed, phydev->duplex);
 
 		phy_modify(phydev, MII_BMCR, ~0, ctl);
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
index 7c51daecf18e..2024d8ef36d9 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -64,15 +64,16 @@ static int xgmiitorgmii_read_status(struct phy_device *phydev)
 	return 0;
 }
 
-static int xgmiitorgmii_set_loopback(struct phy_device *phydev, bool enable)
+static int xgmiitorgmii_set_loopback(struct phy_device *phydev, bool enable,
+				     int speed)
 {
 	struct gmii2rgmii *priv = mdiodev_get_drvdata(&phydev->mdio);
 	int err;
 
 	if (priv->phy_drv->set_loopback)
-		err = priv->phy_drv->set_loopback(phydev, enable);
+		err = priv->phy_drv->set_loopback(phydev, enable, speed);
 	else
-		err = genphy_loopback(phydev, enable);
+		err = genphy_loopback(phydev, enable, speed);
 	if (err < 0)
 		return err;
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7bfbae51070a..4042a1df0c48 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1171,8 +1171,16 @@ struct phy_driver {
 	int (*set_tunable)(struct phy_device *dev,
 			    struct ethtool_tunable *tuna,
 			    const void *data);
-	/** @set_loopback: Set the loopback mood of the PHY */
-	int (*set_loopback)(struct phy_device *dev, bool enable);
+	/**
+	 * @set_loopback: Set the loopback mode of the PHY
+	 * enable selects if the loopback mode is enabled or disabled. If the
+	 * loopback mode is enabled, then the speed of the loopback mode can be
+	 * requested with the speed argument. If the speed argument is zero,
+	 * then any speed can be selected. If the speed argument is > 0, then
+	 * this speed shall be selected for the loopback mode or EOPNOTSUPP
+	 * shall be returned if speed selection is not supported.
+	 */
+	int (*set_loopback)(struct phy_device *dev, bool enable, int speed);
 	/** @get_sqi: Get the signal quality indication */
 	int (*get_sqi)(struct phy_device *dev);
 	/** @get_sqi_max: Get the maximum signal quality indication */
@@ -1965,7 +1973,7 @@ int genphy_read_status(struct phy_device *phydev);
 int genphy_read_master_slave(struct phy_device *phydev);
 int genphy_suspend(struct phy_device *phydev);
 int genphy_resume(struct phy_device *phydev);
-int genphy_loopback(struct phy_device *phydev, bool enable);
+int genphy_loopback(struct phy_device *phydev, bool enable, int speed);
 int genphy_soft_reset(struct phy_device *phydev);
 irqreturn_t genphy_handle_interrupt_no_ack(struct phy_device *phydev);
 
@@ -2007,7 +2015,7 @@ int genphy_c45_pma_baset1_read_master_slave(struct phy_device *phydev);
 int genphy_c45_read_status(struct phy_device *phydev);
 int genphy_c45_baset1_read_status(struct phy_device *phydev);
 int genphy_c45_config_aneg(struct phy_device *phydev);
-int genphy_c45_loopback(struct phy_device *phydev, bool enable);
+int genphy_c45_loopback(struct phy_device *phydev, bool enable, int speed);
 int genphy_c45_pma_resume(struct phy_device *phydev);
 int genphy_c45_pma_suspend(struct phy_device *phydev);
 int genphy_c45_fast_retrain(struct phy_device *phydev, bool enable);
-- 
2.39.5

[PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Gerhard Engleder]

phy_loopback() leaves it to the PHY driver to select the speed of the
loopback mode. Thus, the speed of the loopback mode depends on the PHY
driver in use.

Add support for speed selection to phy_loopback() to enable loopback
with defined speeds. Ensure that link up is signaled if speed changes
as speed is not allowed to change during link up. Link down and up is
necessary for a new speed.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep_main.c    |  2 +-
 .../net/ethernet/hisilicon/hns/hns_ethtool.c  |  4 +-
 .../hisilicon/hns3/hns3pf/hclge_main.c        |  4 +-
 .../hisilicon/hns3/hns3pf/hclge_mdio.c        |  2 +-
 .../stmicro/stmmac/stmmac_selftests.c         |  8 +-
 drivers/net/phy/phy.c                         | 87 +++++++++++++++++++
 drivers/net/phy/phy_device.c                  | 35 --------
 include/linux/phy.h                           |  2 +-
 net/core/selftests.c                          |  4 +-
 9 files changed, 100 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index 13007ac2d6b6..48b279fb73ac 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -230,7 +230,7 @@ static int tsnep_phy_loopback(struct tsnep_adapter *adapter, bool enable)
 {
 	int retval;
 
-	retval = phy_loopback(adapter->phydev, enable);
+	retval = phy_loopback(adapter->phydev, enable, 0);
 
 	/* PHY link state change is not signaled if loopback is enabled, it
 	 * would delay a working loopback anyway, let's ensure that loopback
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 6c458f037262..60a586a951a0 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
@@ -266,9 +266,9 @@ static int hns_nic_config_phy_loopback(struct phy_device *phy_dev, u8 en)
 		if (err)
 			goto out;
 
-		err = phy_loopback(phy_dev, true);
+		err = phy_loopback(phy_dev, true, 0);
 	} else {
-		err = phy_loopback(phy_dev, false);
+		err = phy_loopback(phy_dev, false, 0);
 		if (err)
 			goto out;
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 3f17b3073e50..f8161d6eb152 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -7875,7 +7875,7 @@ static int hclge_enable_phy_loopback(struct hclge_dev *hdev,
 	if (ret)
 		return ret;
 
-	return phy_loopback(phydev, true);
+	return phy_loopback(phydev, true, 0);
 }
 
 static int hclge_disable_phy_loopback(struct hclge_dev *hdev,
@@ -7883,7 +7883,7 @@ static int hclge_disable_phy_loopback(struct hclge_dev *hdev,
 {
 	int ret;
 
-	ret = phy_loopback(phydev, false);
+	ret = phy_loopback(phydev, false, 0);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
index 80079657afeb..9a456ebf9b7c 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
@@ -258,7 +258,7 @@ void hclge_mac_start_phy(struct hclge_dev *hdev)
 	if (!phydev)
 		return;
 
-	phy_loopback(phydev, false);
+	phy_loopback(phydev, false, 0);
 
 	phy_start(phydev);
 }
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index 3ca1c2a816ff..a01bc394d1ac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -382,14 +382,14 @@ static int stmmac_test_phy_loopback(struct stmmac_priv *priv)
 	if (!priv->dev->phydev)
 		return -EOPNOTSUPP;
 
-	ret = phy_loopback(priv->dev->phydev, true);
+	ret = phy_loopback(priv->dev->phydev, true, 0);
 	if (ret)
 		return ret;
 
 	attr.dst = priv->dev->dev_addr;
 	ret = __stmmac_test_loopback(priv, &attr);
 
-	phy_loopback(priv->dev->phydev, false);
+	phy_loopback(priv->dev->phydev, false, 0);
 	return ret;
 }
 
@@ -1985,7 +1985,7 @@ void stmmac_selftest_run(struct net_device *dev,
 		case STMMAC_LOOPBACK_PHY:
 			ret = -EOPNOTSUPP;
 			if (dev->phydev)
-				ret = phy_loopback(dev->phydev, true);
+				ret = phy_loopback(dev->phydev, true, 0);
 			if (!ret)
 				break;
 			fallthrough;
@@ -2018,7 +2018,7 @@ void stmmac_selftest_run(struct net_device *dev,
 		case STMMAC_LOOPBACK_PHY:
 			ret = -EOPNOTSUPP;
 			if (dev->phydev)
-				ret = phy_loopback(dev->phydev, false);
+				ret = phy_loopback(dev->phydev, false, 0);
 			if (!ret)
 				break;
 			fallthrough;
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 16ffc00b419c..a59a8eb3c5e9 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1724,6 +1724,93 @@ void phy_mac_interrupt(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_mac_interrupt);
 
+/**
+ * phy_loopback - Configure loopback mode of PHY
+ * @phydev: target phy_device struct
+ * @enable: enable or disable loopback mode
+ * @speed: enable loopback mode with speed
+ *
+ * Configure loopback mode of PHY and signal link down and link up if speed is
+ * changing.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int phy_loopback(struct phy_device *phydev, bool enable, int speed)
+{
+	bool link_up = false;
+	int ret = 0;
+
+	if (!phydev->drv)
+		return -EIO;
+
+	mutex_lock(&phydev->lock);
+
+	if (enable && phydev->loopback_enabled) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	if (!enable && !phydev->loopback_enabled) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (enable) {
+		/*
+		 * Link up is signaled with a defined speed. If speed changes,
+		 * then first link down and after that link up needs to be
+		 * signaled.
+		 */
+		if (phydev->link && phydev->state == PHY_RUNNING) {
+			/* link is up and signaled */
+			if (speed && phydev->speed != speed) {
+				/* signal link down and up for new speed */
+				phydev->link = false;
+				phydev->state = PHY_NOLINK;
+				phy_link_down(phydev);
+
+				link_up = true;
+			}
+		} else {
+			/* link is not signaled */
+			if (speed) {
+				/* signal link up for new speed */
+				link_up = true;
+			}
+		}
+	}
+
+	if (phydev->drv->set_loopback)
+		ret = phydev->drv->set_loopback(phydev, enable, speed);
+	else
+		ret = genphy_loopback(phydev, enable, speed);
+
+	if (ret) {
+		if (enable) {
+			/* try to restore link if enabling loopback fails */
+			if (phydev->drv->set_loopback)
+				phydev->drv->set_loopback(phydev, false, 0);
+			else
+				genphy_loopback(phydev, false, 0);
+		}
+
+		goto out;
+	}
+
+	if (link_up) {
+		phydev->link = true;
+		phydev->state = PHY_RUNNING;
+		phy_link_up(phydev);
+	}
+
+	phydev->loopback_enabled = enable;
+
+out:
+	mutex_unlock(&phydev->lock);
+	return ret;
+}
+EXPORT_SYMBOL(phy_loopback);
+
 /**
  * phy_eee_tx_clock_stop_capable() - indicate whether the MAC can stop tx clock
  * @phydev: target phy_device struct
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 77c39f7ed340..b58b758a6cce 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2054,41 +2054,6 @@ int phy_resume(struct phy_device *phydev)
 }
 EXPORT_SYMBOL(phy_resume);
 
-int phy_loopback(struct phy_device *phydev, bool enable)
-{
-	int ret = 0;
-
-	if (!phydev->drv)
-		return -EIO;
-
-	mutex_lock(&phydev->lock);
-
-	if (enable && phydev->loopback_enabled) {
-		ret = -EBUSY;
-		goto out;
-	}
-
-	if (!enable && !phydev->loopback_enabled) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (phydev->drv->set_loopback)
-		ret = phydev->drv->set_loopback(phydev, enable, 0);
-	else
-		ret = genphy_loopback(phydev, enable, 0);
-
-	if (ret)
-		goto out;
-
-	phydev->loopback_enabled = enable;
-
-out:
-	mutex_unlock(&phydev->lock);
-	return ret;
-}
-EXPORT_SYMBOL(phy_loopback);
-
 /**
  * phy_reset_after_clk_enable - perform a PHY reset if needed
  * @phydev: target phy_device struct
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 4042a1df0c48..affd23447b4b 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1858,7 +1858,7 @@ int phy_init_hw(struct phy_device *phydev);
 int phy_suspend(struct phy_device *phydev);
 int phy_resume(struct phy_device *phydev);
 int __phy_resume(struct phy_device *phydev);
-int phy_loopback(struct phy_device *phydev, bool enable);
+int phy_loopback(struct phy_device *phydev, bool enable, int speed);
 int phy_sfp_connect_phy(void *upstream, struct phy_device *phy);
 void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy);
 void phy_sfp_attach(void *upstream, struct sfp_bus *bus);
diff --git a/net/core/selftests.c b/net/core/selftests.c
index 8f801e6e3b91..e99ae983fca9 100644
--- a/net/core/selftests.c
+++ b/net/core/selftests.c
@@ -299,7 +299,7 @@ static int net_test_phy_loopback_enable(struct net_device *ndev)
 	if (!ndev->phydev)
 		return -EOPNOTSUPP;
 
-	return phy_loopback(ndev->phydev, true);
+	return phy_loopback(ndev->phydev, true, 0);
 }
 
 static int net_test_phy_loopback_disable(struct net_device *ndev)
@@ -307,7 +307,7 @@ static int net_test_phy_loopback_disable(struct net_device *ndev)
 	if (!ndev->phydev)
 		return -EOPNOTSUPP;
 
-	return phy_loopback(ndev->phydev, false);
+	return phy_loopback(ndev->phydev, false, 0);
 }
 
 static int net_test_phy_loopback_udp(struct net_device *ndev)
-- 
2.39.5

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Jakub Kicinski]

On Thu, 27 Feb 2025 21:31:32 +0100 Gerhard Engleder wrote:
> phy_loopback() leaves it to the PHY driver to select the speed of the
> loopback mode. Thus, the speed of the loopback mode depends on the PHY
> driver in use.
> 
> Add support for speed selection to phy_loopback() to enable loopback
> with defined speeds. Ensure that link up is signaled if speed changes
> as speed is not allowed to change during link up. Link down and up is
> necessary for a new speed.

Hi Andrew, does this one look good to you? I see your review tags 
on other patches :)

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Andrew Lunn]

On Mon, Mar 03, 2025 at 05:35:00PM -0800, Jakub Kicinski wrote:
> On Thu, 27 Feb 2025 21:31:32 +0100 Gerhard Engleder wrote:
> > phy_loopback() leaves it to the PHY driver to select the speed of the
> > loopback mode. Thus, the speed of the loopback mode depends on the PHY
> > driver in use.
> > 
> > Add support for speed selection to phy_loopback() to enable loopback
> > with defined speeds. Ensure that link up is signaled if speed changes
> > as speed is not allowed to change during link up. Link down and up is
> > necessary for a new speed.
> 
> Hi Andrew, does this one look good to you? I see your review tags 
> on other patches :)

Sorry, dropped the ball on this patchset.

I actually think there needs to be a step back and look at the
requirements and what others are doing in the area.

Jijie Shao is doing similar things:
https://lore.kernel.org/lkml/20250213035529.2402283-3-shaojijie@huawei.com/

Both run into the same limitations of the current code, being able to
compose tests from what we currently have.

And then there is what Lee Trager is doing:
https://netdevconf.info/0x19/sessions/talk/open-source-tooling-for-phy-management-and-testing.html

There is some overlap with the work Lee is doing, i assume he will
need to configure the link mode the PCS is using, which is the
scalability issue you raised with previous versions of this patchset.
And configuring the PRBS in the PCS is not so different to configuring
the packet generators we have in net/core/selftest.c.

The current IOCTL interface is definitely too limiting for what Lee
will need. So there is a netlink API coming soon. Should Gerhard and
Jijie try to shoehorn what they want into the current IOCTL handler,
or help design the netlink API? How can selftest.c be taken apart and
put back together to make it more useful? And should the high level
API for PRBS be exported through it, making it easier to use for any
netdev?

	Andrew

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Jakub Kicinski]

On Tue, 4 Mar 2025 14:20:02 +0100 Andrew Lunn wrote:
> The current IOCTL interface is definitely too limiting for what Lee
> will need. So there is a netlink API coming soon. Should Gerhard and
> Jijie try to shoehorn what they want into the current IOCTL handler,
> or help design the netlink API? How can selftest.c be taken apart and
> put back together to make it more useful? And should the high level
> API for PRBS be exported through it, making it easier to use for any
> netdev?

As we think about this let's keep in mind that selftests are generic,
not PHY-centric. Even if we can pass all link settings in there are
other innumerable params people may want in the future.

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Gerhard Engleder]

On 04.03.25 17:15, Jakub Kicinski wrote:
> On Tue, 4 Mar 2025 14:20:02 +0100 Andrew Lunn wrote:
>> The current IOCTL interface is definitely too limiting for what Lee
>> will need. So there is a netlink API coming soon. Should Gerhard and
>> Jijie try to shoehorn what they want into the current IOCTL handler,
>> or help design the netlink API? How can selftest.c be taken apart and
>> put back together to make it more useful? And should the high level
>> API for PRBS be exported through it, making it easier to use for any
>> netdev?
> 
> As we think about this let's keep in mind that selftests are generic,
> not PHY-centric. Even if we can pass all link settings in there are
> other innumerable params people may want in the future.

My patchset can be divided into two parts:
1) Extend phy_loopback() to select a defined speed
2) Extend tsnep selftests to get some in-kernel test coverage for the
    phy_loopback() extension

This discussion is related to the selftest rework of the second part.
Would it be ok to put the first part into a separate patchset, as this
changes make sense and work even without the selftests?

For the selftests IMO Jijie Shao and me should try to extend
net/core/selftests in a generic way for both drivers. There shall not be
multiple "send, receive and check skb" implementations in various
drivers. Andrew suggested to make the selftests generic enough to let
others benefit. To prove that, Jijie Shao needs to be able to use the
new selftest sets.

For me it is ok to keep back these selftests until a new netlink API is
available. I feel not comfortable to design a new netlink API as I have
no need to make the selftests configurable by user space like Lee
Trager.

So what to do with these selftests?
Hold back until new netlink API?
Rework them to only support the "send, receive and check skb" case
without any PHY stuff and use it in tsnep and hibmcge?
Keep them as they are as new test sets and parameters can be added
in the future if needed?

Gerhard

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Gerhard Engleder]

On 04.03.25 21:00, Gerhard Engleder wrote:
> On 04.03.25 17:15, Jakub Kicinski wrote:
>> On Tue, 4 Mar 2025 14:20:02 +0100 Andrew Lunn wrote:
>>> The current IOCTL interface is definitely too limiting for what Lee
>>> will need. So there is a netlink API coming soon. Should Gerhard and
>>> Jijie try to shoehorn what they want into the current IOCTL handler,
>>> or help design the netlink API? How can selftest.c be taken apart and
>>> put back together to make it more useful? And should the high level
>>> API for PRBS be exported through it, making it easier to use for any
>>> netdev?
>>
>> As we think about this let's keep in mind that selftests are generic,
>> not PHY-centric. Even if we can pass all link settings in there are
>> other innumerable params people may want in the future.
> 
> My patchset can be divided into two parts:
> 1) Extend phy_loopback() to select a defined speed
> 2) Extend tsnep selftests to get some in-kernel test coverage for the
>     phy_loopback() extension
> 
> This discussion is related to the selftest rework of the second part.
> Would it be ok to put the first part into a separate patchset, as this
> changes make sense and work even without the selftests?

Andrew, is it ok to put phy_loopback() extension to a separate patch
set?

Gerhard

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Andrew Lunn]

On Thu, Mar 06, 2025 at 06:58:20AM +0100, Gerhard Engleder wrote:
> On 04.03.25 21:00, Gerhard Engleder wrote:
> > On 04.03.25 17:15, Jakub Kicinski wrote:
> > > On Tue, 4 Mar 2025 14:20:02 +0100 Andrew Lunn wrote:
> > > > The current IOCTL interface is definitely too limiting for what Lee
> > > > will need. So there is a netlink API coming soon. Should Gerhard and
> > > > Jijie try to shoehorn what they want into the current IOCTL handler,
> > > > or help design the netlink API? How can selftest.c be taken apart and
> > > > put back together to make it more useful? And should the high level
> > > > API for PRBS be exported through it, making it easier to use for any
> > > > netdev?
> > > 
> > > As we think about this let's keep in mind that selftests are generic,
> > > not PHY-centric. Even if we can pass all link settings in there are
> > > other innumerable params people may want in the future.
> > 
> > My patchset can be divided into two parts:
> > 1) Extend phy_loopback() to select a defined speed
> > 2) Extend tsnep selftests to get some in-kernel test coverage for the
> >     phy_loopback() extension
> > 
> > This discussion is related to the selftest rework of the second part.
> > Would it be ok to put the first part into a separate patchset, as this
> > changes make sense and work even without the selftests?
> 
> Andrew, is it ok to put phy_loopback() extension to a separate patch
> set?

Without the selftest part, the phy loopback changes go unused. We
don't normally add APIs without a user. So i would say no, it should
be all or nothing. I don't think it will cause many problems if these
patches need to wait a while, a rebase should be easy, this area of
phylib is pretty stable.

	Andrew

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Gerhard Engleder]

On 07.03.25 17:27, Andrew Lunn wrote:
> On Thu, Mar 06, 2025 at 06:58:20AM +0100, Gerhard Engleder wrote:
>> On 04.03.25 21:00, Gerhard Engleder wrote:
>>> On 04.03.25 17:15, Jakub Kicinski wrote:
>>>> On Tue, 4 Mar 2025 14:20:02 +0100 Andrew Lunn wrote:
>>>>> The current IOCTL interface is definitely too limiting for what Lee
>>>>> will need. So there is a netlink API coming soon. Should Gerhard and
>>>>> Jijie try to shoehorn what they want into the current IOCTL handler,
>>>>> or help design the netlink API? How can selftest.c be taken apart and
>>>>> put back together to make it more useful? And should the high level
>>>>> API for PRBS be exported through it, making it easier to use for any
>>>>> netdev?
>>>>
>>>> As we think about this let's keep in mind that selftests are generic,
>>>> not PHY-centric. Even if we can pass all link settings in there are
>>>> other innumerable params people may want in the future.
>>>
>>> My patchset can be divided into two parts:
>>> 1) Extend phy_loopback() to select a defined speed
>>> 2) Extend tsnep selftests to get some in-kernel test coverage for the
>>>      phy_loopback() extension
>>>
>>> This discussion is related to the selftest rework of the second part.
>>> Would it be ok to put the first part into a separate patchset, as this
>>> changes make sense and work even without the selftests?
>>
>> Andrew, is it ok to put phy_loopback() extension to a separate patch
>> set?
> 
> Without the selftest part, the phy loopback changes go unused. We
> don't normally add APIs without a user. So i would say no, it should
> be all or nothing. I don't think it will cause many problems if these
> patches need to wait a while, a rebase should be easy, this area of
> phylib is pretty stable.

Why no user? The tsnep driver is the user to get loopback again working
after 6ff3cddc365b ("net: phylib: do not disable autoneg for fixed
speeds >= 1G"). The phy_loopback changes are used by
tsnep_phy_loopback().

Thanks to your comments it is also an improvement of the loopback
behavior, as now loopback signals the new speed like a normal link up.

Gerhard

Re: [PATCH net-next v9 2/8] net: phy: Support speed selection for PHY loopback

[Gerhard Engleder]

On 07.03.25 20:15, Gerhard Engleder wrote:
> On 07.03.25 17:27, Andrew Lunn wrote:
>> On Thu, Mar 06, 2025 at 06:58:20AM +0100, Gerhard Engleder wrote:
>>> On 04.03.25 21:00, Gerhard Engleder wrote:
>>>> On 04.03.25 17:15, Jakub Kicinski wrote:
>>>>> On Tue, 4 Mar 2025 14:20:02 +0100 Andrew Lunn wrote:
>>>>>> The current IOCTL interface is definitely too limiting for what Lee
>>>>>> will need. So there is a netlink API coming soon. Should Gerhard and
>>>>>> Jijie try to shoehorn what they want into the current IOCTL handler,
>>>>>> or help design the netlink API? How can selftest.c be taken apart and
>>>>>> put back together to make it more useful? And should the high level
>>>>>> API for PRBS be exported through it, making it easier to use for any
>>>>>> netdev?
>>>>>
>>>>> As we think about this let's keep in mind that selftests are generic,
>>>>> not PHY-centric. Even if we can pass all link settings in there are
>>>>> other innumerable params people may want in the future.
>>>>
>>>> My patchset can be divided into two parts:
>>>> 1) Extend phy_loopback() to select a defined speed
>>>> 2) Extend tsnep selftests to get some in-kernel test coverage for the
>>>>      phy_loopback() extension
>>>>
>>>> This discussion is related to the selftest rework of the second part.
>>>> Would it be ok to put the first part into a separate patchset, as this
>>>> changes make sense and work even without the selftests?
>>>
>>> Andrew, is it ok to put phy_loopback() extension to a separate patch
>>> set?
>>
>> Without the selftest part, the phy loopback changes go unused. We
>> don't normally add APIs without a user. So i would say no, it should
>> be all or nothing. I don't think it will cause many problems if these
>> patches need to wait a while, a rebase should be easy, this area of
>> phylib is pretty stable.
> 
> Why no user? The tsnep driver is the user to get loopback again working
> after 6ff3cddc365b ("net: phylib: do not disable autoneg for fixed
> speeds >= 1G"). The phy_loopback changes are used by
> tsnep_phy_loopback().
> 
> Thanks to your comments it is also an improvement of the loopback
> behavior, as now loopback signals the new speed like a normal link up.

Is it ok for you the put the phy_loopback() extension with loopback fix
for the tsnep driver to a separate patch set?

Gerhard

[PATCH net-next v9 3/8] net: phy: micrel: Add loopback support

[Gerhard Engleder]

The KSZ9031 PHYs requires full duplex for loopback mode. Add PHY
specific set_loopback() to ensure this.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/phy/micrel.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 9c0b1c229af6..f375832e3987 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1030,6 +1030,29 @@ static int ksz9021_config_init(struct phy_device *phydev)
 #define MII_KSZ9031RN_EDPD		0x23
 #define MII_KSZ9031RN_EDPD_ENABLE	BIT(0)
 
+static int ksz9031_set_loopback(struct phy_device *phydev, bool enable,
+				int speed)
+{
+	u16 ctl = BMCR_LOOPBACK;
+	int val;
+
+	if (!enable)
+		return genphy_loopback(phydev, enable, 0);
+
+	if (speed == SPEED_10 || speed == SPEED_100 || speed == SPEED_1000)
+		phydev->speed = speed;
+	else if (speed)
+		return -EINVAL;
+	phydev->duplex = DUPLEX_FULL;
+
+	ctl |= mii_bmcr_encode_fixed(phydev->speed, phydev->duplex);
+
+	phy_write(phydev, MII_BMCR, ctl);
+
+	return phy_read_poll_timeout(phydev, MII_BMSR, val, val & BMSR_LSTATUS,
+				     5000, 500000, true);
+}
+
 static int ksz9031_of_load_skew_values(struct phy_device *phydev,
 				       const struct device_node *of_node,
 				       u16 reg, size_t field_sz,
@@ -5564,6 +5587,7 @@ static struct phy_driver ksphy_driver[] = {
 	.resume		= kszphy_resume,
 	.cable_test_start	= ksz9x31_cable_test_start,
 	.cable_test_get_status	= ksz9x31_cable_test_get_status,
+	.set_loopback	= ksz9031_set_loopback,
 }, {
 	.phy_id		= PHY_ID_LAN8814,
 	.phy_id_mask	= MICREL_PHY_ID_MASK,
-- 
2.39.5

[PATCH net-next v9 4/8] net: phy: marvell: Align set_loopback() implementation

[Gerhard Engleder]

Use genphy_loopback() to disable loopback like ksz9031_set_loopback().
This way disable loopback is implemented only once within
genphy_loopback() and the set_loopback() implementations look similar.

Also fix comment about msleep() in the out-of loopback case which is not
executed in the out-of loopback case.

Suggested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/marvell.c | 72 ++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index f2ad675537d1..623292948fa7 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -2133,56 +2133,50 @@ static void marvell_get_stats_simple(struct phy_device *phydev,
 
 static int m88e1510_loopback(struct phy_device *phydev, bool enable, int speed)
 {
+	u16 bmcr_ctl, mscr2_ctl = 0;
 	int err;
 
-	if (enable) {
-		u16 bmcr_ctl, mscr2_ctl = 0;
+	if (!enable)
+		return genphy_loopback(phydev, enable, 0);
 
-		if (speed == SPEED_10 || speed == SPEED_100 ||
-		    speed == SPEED_1000)
-			phydev->speed = speed;
-		else if (speed)
-			return -EINVAL;
-
-		bmcr_ctl = mii_bmcr_encode_fixed(phydev->speed, phydev->duplex);
-
-		err = phy_write(phydev, MII_BMCR, bmcr_ctl);
-		if (err < 0)
-			return err;
+	if (speed == SPEED_10 || speed == SPEED_100 || speed == SPEED_1000)
+		phydev->speed = speed;
+	else if (speed)
+		return -EINVAL;
 
-		if (phydev->speed == SPEED_1000)
-			mscr2_ctl = BMCR_SPEED1000;
-		else if (phydev->speed == SPEED_100)
-			mscr2_ctl = BMCR_SPEED100;
+	bmcr_ctl = mii_bmcr_encode_fixed(phydev->speed, phydev->duplex);
 
-		err = phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
-				       MII_88E1510_MSCR_2, BMCR_SPEED1000 |
-				       BMCR_SPEED100, mscr2_ctl);
-		if (err < 0)
-			return err;
+	err = phy_write(phydev, MII_BMCR, bmcr_ctl);
+	if (err < 0)
+		return err;
 
-		/* Need soft reset to have speed configuration takes effect */
-		err = genphy_soft_reset(phydev);
-		if (err < 0)
-			return err;
+	if (phydev->speed == SPEED_1000)
+		mscr2_ctl = BMCR_SPEED1000;
+	else if (phydev->speed == SPEED_100)
+		mscr2_ctl = BMCR_SPEED100;
 
-		err = phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
-				 BMCR_LOOPBACK);
+	err = phy_modify_paged(phydev, MII_MARVELL_MSCR_PAGE,
+			       MII_88E1510_MSCR_2, BMCR_SPEED1000 |
+			       BMCR_SPEED100, mscr2_ctl);
+	if (err < 0)
+		return err;
 
-		if (!err) {
-			/* It takes some time for PHY device to switch
-			 * into/out-of loopback mode.
-			 */
-			msleep(1000);
-		}
+	/* Need soft reset to have speed configuration takes effect */
+	err = genphy_soft_reset(phydev);
+	if (err < 0)
 		return err;
-	} else {
-		err = phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, 0);
-		if (err < 0)
-			return err;
 
-		return phy_config_aneg(phydev);
+	err = phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
+			 BMCR_LOOPBACK);
+
+	if (!err) {
+		/*
+		 * It takes some time for PHY device to switch into loopback
+		 * mode.
+		 */
+		msleep(1000);
 	}
+	return err;
 }
 
 static int marvell_vct5_wait_complete(struct phy_device *phydev)
-- 
2.39.5

[PATCH net-next v9 5/8] tsnep: Select speed for loopback

[Gerhard Engleder]

Use 100 Mbps only if the PHY is configured to this speed. Otherwise use
always the maximum speed of 1000 Mbps.

Also remove explicit setting of carrier on and link mode after loopback.
This is not needed anymore, because phy_loopback() with selected speed
signals the link and the speed to the MAC.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/engleder/tsnep_main.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index 48b279fb73ac..61a23413b577 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -228,20 +228,19 @@ static void tsnep_phy_link_status_change(struct net_device *netdev)
 
 static int tsnep_phy_loopback(struct tsnep_adapter *adapter, bool enable)
 {
-	int retval;
-
-	retval = phy_loopback(adapter->phydev, enable, 0);
+	int speed;
 
-	/* PHY link state change is not signaled if loopback is enabled, it
-	 * would delay a working loopback anyway, let's ensure that loopback
-	 * is working immediately by setting link mode directly
-	 */
-	if (!retval && enable) {
-		netif_carrier_on(adapter->netdev);
-		tsnep_set_link_mode(adapter);
+	if (enable) {
+		if (adapter->phydev->autoneg == AUTONEG_DISABLE &&
+		    adapter->phydev->speed == SPEED_100)
+			speed = SPEED_100;
+		else
+			speed = SPEED_1000;
+	} else {
+		speed = 0;
 	}
 
-	return retval;
+	return phy_loopback(adapter->phydev, enable, speed);
 }
 
 static int tsnep_phy_open(struct tsnep_adapter *adapter)
-- 
2.39.5

[PATCH net-next v9 6/8] net: selftests: Support selftest sets

[Gerhard Engleder]

Currently only one test set is supported. Extend selftests to support
multiple test sets with different speeds which can be combined by network
drivers.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
CC: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/net/selftests.h |  27 +++++++
 net/core/selftests.c    | 171 ++++++++++++++++++++++++++--------------
 2 files changed, 141 insertions(+), 57 deletions(-)

diff --git a/include/net/selftests.h b/include/net/selftests.h
index e65e8d230d33..23721815f4bd 100644
--- a/include/net/selftests.h
+++ b/include/net/selftests.h
@@ -4,6 +4,14 @@
 
 #include <linux/ethtool.h>
 
+/**
+ * enum net_selftest - selftest set ID
+ * @NET_SELFTEST_LOOPBACK_CARRIER: Loopback tests based on carrier speed
+ */
+enum net_selftest {
+	NET_SELFTEST_LOOPBACK_CARRIER = 0,
+};
+
 #if IS_ENABLED(CONFIG_NET_SELFTESTS)
 
 void net_selftest(struct net_device *ndev, struct ethtool_test *etest,
@@ -11,6 +19,11 @@ void net_selftest(struct net_device *ndev, struct ethtool_test *etest,
 int net_selftest_get_count(void);
 void net_selftest_get_strings(u8 *data);
 
+void net_selftest_set(int set, int speed, struct net_device *ndev,
+		      struct ethtool_test *etest, u64 *buf);
+int net_selftest_set_get_count(int set);
+void net_selftest_set_get_strings(int set, int speed, u8 **data);
+
 #else
 
 static inline void net_selftest(struct net_device *ndev, struct ethtool_test *etest,
@@ -27,5 +40,19 @@ static inline void net_selftest_get_strings(u8 *data)
 {
 }
 
+static inline void net_selftest_set(int set, int speed, struct net_device *ndev,
+				    struct ethtool_test *etest, u64 *buf)
+{
+}
+
+static inline int net_selftest_set_get_count(int set)
+{
+	return 0;
+}
+
+static inline void net_selftest_set_get_strings(int set, int speed, u8 **data)
+{
+}
+
 #endif
 #endif /* _NET_SELFTESTS */
diff --git a/net/core/selftests.c b/net/core/selftests.c
index e99ae983fca9..ec9bb149a378 100644
--- a/net/core/selftests.c
+++ b/net/core/selftests.c
@@ -14,6 +14,10 @@
 #include <net/tcp.h>
 #include <net/udp.h>
 
+struct net_test_ctx {
+	u8 next_id;
+};
+
 struct net_packet_attrs {
 	const unsigned char *src;
 	const unsigned char *dst;
@@ -44,14 +48,13 @@ struct netsfhdr {
 	u8 id;
 } __packed;
 
-static u8 net_test_next_id;
-
 #define NET_TEST_PKT_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr) + \
 			   sizeof(struct netsfhdr))
 #define NET_TEST_PKT_MAGIC	0xdeadcafecafedeadULL
 #define NET_LB_TIMEOUT		msecs_to_jiffies(200)
 
-static struct sk_buff *net_test_get_skb(struct net_device *ndev,
+static struct sk_buff *net_test_get_skb(struct net_test_ctx *ctx,
+					struct net_device *ndev,
 					struct net_packet_attrs *attr)
 {
 	struct sk_buff *skb = NULL;
@@ -141,8 +144,8 @@ static struct sk_buff *net_test_get_skb(struct net_device *ndev,
 	shdr = skb_put(skb, sizeof(*shdr));
 	shdr->version = 0;
 	shdr->magic = cpu_to_be64(NET_TEST_PKT_MAGIC);
-	attr->id = net_test_next_id;
-	shdr->id = net_test_next_id++;
+	attr->id = ctx->next_id;
+	shdr->id = ctx->next_id++;
 
 	if (attr->size)
 		skb_put(skb, attr->size);
@@ -237,7 +240,8 @@ static int net_test_loopback_validate(struct sk_buff *skb,
 	return 0;
 }
 
-static int __net_test_loopback(struct net_device *ndev,
+static int __net_test_loopback(struct net_test_ctx *ctx,
+			       struct net_device *ndev,
 			       struct net_packet_attrs *attr)
 {
 	struct net_test_priv *tpriv;
@@ -258,7 +262,7 @@ static int __net_test_loopback(struct net_device *ndev,
 	tpriv->packet = attr;
 	dev_add_pack(&tpriv->pt);
 
-	skb = net_test_get_skb(ndev, attr);
+	skb = net_test_get_skb(ctx, ndev, attr);
 	if (!skb) {
 		ret = -ENOMEM;
 		goto cleanup;
@@ -284,17 +288,40 @@ static int __net_test_loopback(struct net_device *ndev,
 	return ret;
 }
 
-static int net_test_netif_carrier(struct net_device *ndev)
+struct net_test {
+	const char *name;
+	int (*fn)(struct net_test_ctx *ctx, struct net_device *ndev);
+};
+
+/**
+ * NET_SELFTEST - Define a selftest.
+ * @_name: Selftest name.
+ * @_string: Selftest string.
+ */
+#define NET_SELFTEST(_name, _string)		\
+	struct net_test net_test_##_name = {	\
+		.name = _string,		\
+		.fn = net_test_##_name##_fn,	\
+	}
+
+static int net_test_netif_carrier_fn(struct net_test_ctx *ctx,
+				     struct net_device *ndev)
 {
 	return netif_carrier_ok(ndev) ? 0 : -ENOLINK;
 }
 
-static int net_test_phy_phydev(struct net_device *ndev)
+static const NET_SELFTEST(netif_carrier, "Carrier");
+
+static int net_test_phy_phydev_fn(struct net_test_ctx *ctx,
+				  struct net_device *ndev)
 {
 	return ndev->phydev ? 0 : -EOPNOTSUPP;
 }
 
-static int net_test_phy_loopback_enable(struct net_device *ndev)
+static const NET_SELFTEST(phy_phydev, "PHY dev is present");
+
+static int net_test_phy_loopback_enable_fn(struct net_test_ctx *ctx,
+					   struct net_device *ndev)
 {
 	if (!ndev->phydev)
 		return -EOPNOTSUPP;
@@ -302,7 +329,10 @@ static int net_test_phy_loopback_enable(struct net_device *ndev)
 	return phy_loopback(ndev->phydev, true, 0);
 }
 
-static int net_test_phy_loopback_disable(struct net_device *ndev)
+static const NET_SELFTEST(phy_loopback_enable, "PHY loopback enable");
+
+static int net_test_phy_loopback_disable_fn(struct net_test_ctx *ctx,
+					    struct net_device *ndev)
 {
 	if (!ndev->phydev)
 		return -EOPNOTSUPP;
@@ -310,98 +340,125 @@ static int net_test_phy_loopback_disable(struct net_device *ndev)
 	return phy_loopback(ndev->phydev, false, 0);
 }
 
-static int net_test_phy_loopback_udp(struct net_device *ndev)
+static const NET_SELFTEST(phy_loopback_disable, "PHY loopback disable");
+
+static int net_test_phy_loopback_udp_fn(struct net_test_ctx *ctx,
+					struct net_device *ndev)
 {
 	struct net_packet_attrs attr = { };
 
 	attr.dst = ndev->dev_addr;
-	return __net_test_loopback(ndev, &attr);
+	return __net_test_loopback(ctx, ndev, &attr);
 }
 
-static int net_test_phy_loopback_udp_mtu(struct net_device *ndev)
+static const NET_SELFTEST(phy_loopback_udp, "PHY loopback UDP");
+
+static int net_test_phy_loopback_udp_mtu_fn(struct net_test_ctx *ctx,
+					    struct net_device *ndev)
 {
 	struct net_packet_attrs attr = { };
 
 	attr.dst = ndev->dev_addr;
 	attr.max_size = ndev->mtu;
-	return __net_test_loopback(ndev, &attr);
+	return __net_test_loopback(ctx, ndev, &attr);
 }
 
-static int net_test_phy_loopback_tcp(struct net_device *ndev)
+static const NET_SELFTEST(phy_loopback_udp_mtu, "PHY loopback MTU");
+
+static int net_test_phy_loopback_tcp_fn(struct net_test_ctx *ctx,
+					struct net_device *ndev)
 {
 	struct net_packet_attrs attr = { };
 
 	attr.dst = ndev->dev_addr;
 	attr.tcp = true;
-	return __net_test_loopback(ndev, &attr);
+	return __net_test_loopback(ctx, ndev, &attr);
 }
 
-static const struct net_test {
-	char name[ETH_GSTRING_LEN];
-	int (*fn)(struct net_device *ndev);
-} net_selftests[] = {
-	{
-		.name = "Carrier                       ",
-		.fn = net_test_netif_carrier,
-	}, {
-		.name = "PHY dev is present            ",
-		.fn = net_test_phy_phydev,
-	}, {
-		/* This test should be done before all PHY loopback test */
-		.name = "PHY internal loopback, enable ",
-		.fn = net_test_phy_loopback_enable,
-	}, {
-		.name = "PHY internal loopback, UDP    ",
-		.fn = net_test_phy_loopback_udp,
-	}, {
-		.name = "PHY internal loopback, MTU    ",
-		.fn = net_test_phy_loopback_udp_mtu,
-	}, {
-		.name = "PHY internal loopback, TCP    ",
-		.fn = net_test_phy_loopback_tcp,
-	}, {
-		/* This test should be done after all PHY loopback test */
-		.name = "PHY internal loopback, disable",
-		.fn = net_test_phy_loopback_disable,
-	},
+static const NET_SELFTEST(phy_loopback_tcp, "PHY loopback TCP");
+
+static const struct net_test *net_selftests_carrier[] = {
+	&net_test_netif_carrier,
+	&net_test_phy_phydev,
+	&net_test_phy_loopback_enable,
+	&net_test_phy_loopback_udp,
+	&net_test_phy_loopback_udp_mtu,
+	&net_test_phy_loopback_tcp,
+	&net_test_phy_loopback_disable,
 };
 
-void net_selftest(struct net_device *ndev, struct ethtool_test *etest, u64 *buf)
+static const struct net_test **net_selftests_set_get(int set)
 {
-	int count = net_selftest_get_count();
+	switch (set) {
+	case NET_SELFTEST_LOOPBACK_CARRIER:
+		return net_selftests_carrier;
+	}
+
+	return NULL;
+}
+
+void net_selftest_set(int set, int speed, struct net_device *ndev,
+		      struct ethtool_test *etest, u64 *buf)
+{
+	const struct net_test **selftests = net_selftests_set_get(set);
+	int count = net_selftest_set_get_count(set);
+	struct net_test_ctx ctx = { 0 };
 	int i;
 
 	memset(buf, 0, sizeof(*buf) * count);
-	net_test_next_id = 0;
 
-	if (etest->flags != ETH_TEST_FL_OFFLINE) {
+	if (!(etest->flags & ETH_TEST_FL_OFFLINE)) {
 		netdev_err(ndev, "Only offline tests are supported\n");
 		etest->flags |= ETH_TEST_FL_FAILED;
 		return;
 	}
 
-
 	for (i = 0; i < count; i++) {
-		buf[i] = net_selftests[i].fn(ndev);
+		buf[i] = selftests[i]->fn(&ctx, ndev);
 		if (buf[i] && (buf[i] != -EOPNOTSUPP))
 			etest->flags |= ETH_TEST_FL_FAILED;
 	}
 }
+EXPORT_SYMBOL_GPL(net_selftest_set);
+
+int net_selftest_set_get_count(int set)
+{
+	switch (set) {
+	case NET_SELFTEST_LOOPBACK_CARRIER:
+		return ARRAY_SIZE(net_selftests_carrier);
+	default:
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(net_selftest_set_get_count);
+
+void net_selftest_set_get_strings(int set, int speed, u8 **data)
+{
+	const struct net_test **selftests = net_selftests_set_get(set);
+	int count = net_selftest_set_get_count(set);
+	int i;
+
+	/* right pad strings for aligned ethtool output */
+	for (i = 0; i < count; i++)
+		ethtool_sprintf(data, "%-30s", selftests[i]->name);
+}
+EXPORT_SYMBOL_GPL(net_selftest_set_get_strings);
+
+void net_selftest(struct net_device *ndev, struct ethtool_test *etest, u64 *buf)
+{
+	net_selftest_set(NET_SELFTEST_LOOPBACK_CARRIER, 0, ndev, etest, buf);
+}
 EXPORT_SYMBOL_GPL(net_selftest);
 
 int net_selftest_get_count(void)
 {
-	return ARRAY_SIZE(net_selftests);
+	return net_selftest_set_get_count(NET_SELFTEST_LOOPBACK_CARRIER);
 }
 EXPORT_SYMBOL_GPL(net_selftest_get_count);
 
 void net_selftest_get_strings(u8 *data)
 {
-	int i;
-
-	for (i = 0; i < net_selftest_get_count(); i++)
-		ethtool_sprintf(&data, "%2d. %s", i + 1,
-				net_selftests[i].name);
+	net_selftest_set_get_strings(NET_SELFTEST_LOOPBACK_CARRIER, 0, &data);
 }
 EXPORT_SYMBOL_GPL(net_selftest_get_strings);
 
-- 
2.39.5

Re: [PATCH net-next v9 6/8] net: selftests: Support selftest sets

[Jakub Kicinski]

On Thu, 27 Feb 2025 21:31:36 +0100 Gerhard Engleder wrote:
> +/**
> + * enum net_selftest - selftest set ID
> + * @NET_SELFTEST_LOOPBACK_CARRIER: Loopback tests based on carrier speed

why are these "tests based on carrier speed"?
these tests use default parameters AFAICT, I'm not seeing the relevance
of carrier.. Maybe you can explain.

> + */
> +enum net_selftest {
> +	NET_SELFTEST_LOOPBACK_CARRIER = 0,
> +};

> diff --git a/net/core/selftests.c b/net/core/selftests.c
> index e99ae983fca9..ec9bb149a378 100644
> --- a/net/core/selftests.c
> +++ b/net/core/selftests.c
> @@ -14,6 +14,10 @@
>  #include <net/tcp.h>
>  #include <net/udp.h>
>  
> +struct net_test_ctx {
> +	u8 next_id;
> +};
> +
>  struct net_packet_attrs {
>  	const unsigned char *src;
>  	const unsigned char *dst;
> @@ -44,14 +48,13 @@ struct netsfhdr {
>  	u8 id;
>  } __packed;
>  
> -static u8 net_test_next_id;

The removal of the global state seems loosely connected to the rest,
the global state is okay because we hold RTNL across, AFAIU, which
will still be true for tests varying speed. Not using global state
is a worthwhile cleanup IMO, but I think you should split this patch 
in 2.

Re: [PATCH net-next v9 6/8] net: selftests: Support selftest sets

[Gerhard Engleder]

On 04.03.25 02:41, Jakub Kicinski wrote:
> On Thu, 27 Feb 2025 21:31:36 +0100 Gerhard Engleder wrote:
>> +/**
>> + * enum net_selftest - selftest set ID
>> + * @NET_SELFTEST_LOOPBACK_CARRIER: Loopback tests based on carrier speed
> 
> why are these "tests based on carrier speed"?
> these tests use default parameters AFAICT, I'm not seeing the relevance
> of carrier.. Maybe you can explain.

genphy_loopback() and most PHY specific loopback implementations
use the speed of the current carrier to determine the speed of
the to be configured loopback mode. Therefore, a carrier/linkup is
required to execute these tests successfully and I assume that's why
the already existing test set checks for carrier first.

I will enrich the comment of this test set.

> 
>> + */
>> +enum net_selftest {
>> +	NET_SELFTEST_LOOPBACK_CARRIER = 0,
>> +};
> 
>> diff --git a/net/core/selftests.c b/net/core/selftests.c
>> index e99ae983fca9..ec9bb149a378 100644
>> --- a/net/core/selftests.c
>> +++ b/net/core/selftests.c
>> @@ -14,6 +14,10 @@
>>   #include <net/tcp.h>
>>   #include <net/udp.h>
>>   
>> +struct net_test_ctx {
>> +	u8 next_id;
>> +};
>> +
>>   struct net_packet_attrs {
>>   	const unsigned char *src;
>>   	const unsigned char *dst;
>> @@ -44,14 +48,13 @@ struct netsfhdr {
>>   	u8 id;
>>   } __packed;
>>   
>> -static u8 net_test_next_id;
> 
> The removal of the global state seems loosely connected to the rest,
> the global state is okay because we hold RTNL across, AFAIU, which
> will still be true for tests varying speed. Not using global state
> is a worthwhile cleanup IMO, but I think you should split this patch
> in 2.

I will split this into a separate commit.

Thank you for the review!

Gerhard

[PATCH net-next v9 7/8] net: selftests: Add selftests sets with fixed speed

[Gerhard Engleder]

Add PHY loopback selftest set which uses the speed argument to select a
fixed speed.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 include/net/selftests.h |  2 ++
 net/core/selftests.c    | 47 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/include/net/selftests.h b/include/net/selftests.h
index 23721815f4bd..a49d725bff9a 100644
--- a/include/net/selftests.h
+++ b/include/net/selftests.h
@@ -7,9 +7,11 @@
 /**
  * enum net_selftest - selftest set ID
  * @NET_SELFTEST_LOOPBACK_CARRIER: Loopback tests based on carrier speed
+ * @NET_SELFTEST_LOOPBACK_SPEED: Loopback tests with fixed speed
  */
 enum net_selftest {
 	NET_SELFTEST_LOOPBACK_CARRIER = 0,
+	NET_SELFTEST_LOOPBACK_SPEED,
 };
 
 #if IS_ENABLED(CONFIG_NET_SELFTESTS)
diff --git a/net/core/selftests.c b/net/core/selftests.c
index ec9bb149a378..50c2916ff6ec 100644
--- a/net/core/selftests.c
+++ b/net/core/selftests.c
@@ -16,6 +16,7 @@
 
 struct net_test_ctx {
 	u8 next_id;
+	int speed;
 };
 
 struct net_packet_attrs {
@@ -291,6 +292,7 @@ static int __net_test_loopback(struct net_test_ctx *ctx,
 struct net_test {
 	const char *name;
 	int (*fn)(struct net_test_ctx *ctx, struct net_device *ndev);
+	int loopback_speed;
 };
 
 /**
@@ -304,6 +306,16 @@ struct net_test {
 		.fn = net_test_##_name##_fn,	\
 	}
 
+/**
+ * NET_SELFTEST_LOOPBACK_SPEED - Define a selftest which enables loopback with a fixed speed.
+ * @_name: Selftest name.
+ */
+#define NET_SELFTEST_LOOPBACK_SPEED(_name)	\
+	struct net_test net_test_##_name = {	\
+		.fn = net_test_##_name##_fn,	\
+		.loopback_speed = true,		\
+	}
+
 static int net_test_netif_carrier_fn(struct net_test_ctx *ctx,
 				     struct net_device *ndev)
 {
@@ -331,6 +343,17 @@ static int net_test_phy_loopback_enable_fn(struct net_test_ctx *ctx,
 
 static const NET_SELFTEST(phy_loopback_enable, "PHY loopback enable");
 
+static int net_test_phy_loopback_speed_enable_fn(struct net_test_ctx *ctx,
+						 struct net_device *ndev)
+{
+	if (!ndev->phydev)
+		return -EOPNOTSUPP;
+
+	return phy_loopback(ndev->phydev, true, ctx->speed);
+}
+
+static const NET_SELFTEST_LOOPBACK_SPEED(phy_loopback_speed_enable);
+
 static int net_test_phy_loopback_disable_fn(struct net_test_ctx *ctx,
 					    struct net_device *ndev)
 {
@@ -387,11 +410,22 @@ static const struct net_test *net_selftests_carrier[] = {
 	&net_test_phy_loopback_disable,
 };
 
+static const struct net_test *net_selftests_speed[] = {
+	&net_test_phy_phydev,
+	&net_test_phy_loopback_speed_enable,
+	&net_test_phy_loopback_udp,
+	&net_test_phy_loopback_udp_mtu,
+	&net_test_phy_loopback_tcp,
+	&net_test_phy_loopback_disable,
+};
+
 static const struct net_test **net_selftests_set_get(int set)
 {
 	switch (set) {
 	case NET_SELFTEST_LOOPBACK_CARRIER:
 		return net_selftests_carrier;
+	case NET_SELFTEST_LOOPBACK_SPEED:
+		return net_selftests_speed;
 	}
 
 	return NULL;
@@ -402,7 +436,7 @@ void net_selftest_set(int set, int speed, struct net_device *ndev,
 {
 	const struct net_test **selftests = net_selftests_set_get(set);
 	int count = net_selftest_set_get_count(set);
-	struct net_test_ctx ctx = { 0 };
+	struct net_test_ctx ctx = { .speed = speed };
 	int i;
 
 	memset(buf, 0, sizeof(*buf) * count);
@@ -426,6 +460,8 @@ int net_selftest_set_get_count(int set)
 	switch (set) {
 	case NET_SELFTEST_LOOPBACK_CARRIER:
 		return ARRAY_SIZE(net_selftests_carrier);
+	case NET_SELFTEST_LOOPBACK_SPEED:
+		return ARRAY_SIZE(net_selftests_speed);
 	default:
 		return -EINVAL;
 	}
@@ -439,8 +475,13 @@ void net_selftest_set_get_strings(int set, int speed, u8 **data)
 	int i;
 
 	/* right pad strings for aligned ethtool output */
-	for (i = 0; i < count; i++)
-		ethtool_sprintf(data, "%-30s", selftests[i]->name);
+	for (i = 0; i < count; i++) {
+		if (selftests[i]->loopback_speed)
+			ethtool_sprintf(data, "PHY loopback %-17s",
+					phy_speed_to_str(speed));
+		else
+			ethtool_sprintf(data, "%-30s", selftests[i]->name);
+	}
 }
 EXPORT_SYMBOL_GPL(net_selftest_set_get_strings);
 
-- 
2.39.5

[PATCH net-next v9 8/8] tsnep: Add loopback selftests

[Gerhard Engleder]

Add selftest sets with 100 Mbps and 1000 Mbps fixed speed to ethtool
selftests.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 .../net/ethernet/engleder/tsnep_selftests.c   | 30 +++++++++++++++----
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/engleder/tsnep_selftests.c b/drivers/net/ethernet/engleder/tsnep_selftests.c
index 8a9145f93147..b1f08f17f55b 100644
--- a/drivers/net/ethernet/engleder/tsnep_selftests.c
+++ b/drivers/net/ethernet/engleder/tsnep_selftests.c
@@ -4,6 +4,7 @@
 #include "tsnep.h"
 
 #include <net/pkt_sched.h>
+#include <net/selftests.h>
 
 enum tsnep_test {
 	TSNEP_TEST_ENABLE = 0,
@@ -756,27 +757,36 @@ static bool tsnep_test_taprio_extension(struct tsnep_adapter *adapter)
 
 int tsnep_ethtool_get_test_count(void)
 {
-	return TSNEP_TEST_COUNT;
+	int count = TSNEP_TEST_COUNT;
+
+	count += net_selftest_set_get_count(NET_SELFTEST_LOOPBACK_SPEED);
+	count += net_selftest_set_get_count(NET_SELFTEST_LOOPBACK_SPEED);
+
+	return count;
 }
 
 void tsnep_ethtool_get_test_strings(u8 *data)
 {
 	memcpy(data, tsnep_test_strings, sizeof(tsnep_test_strings));
+	data += sizeof(tsnep_test_strings);
+
+	net_selftest_set_get_strings(NET_SELFTEST_LOOPBACK_SPEED, 100, &data);
+	net_selftest_set_get_strings(NET_SELFTEST_LOOPBACK_SPEED, 1000, &data);
 }
 
 void tsnep_ethtool_self_test(struct net_device *netdev,
 			     struct ethtool_test *eth_test, u64 *data)
 {
 	struct tsnep_adapter *adapter = netdev_priv(netdev);
+	int count = tsnep_ethtool_get_test_count();
+	int i;
 
-	eth_test->len = TSNEP_TEST_COUNT;
+	eth_test->len = count;
 
 	if (eth_test->flags != ETH_TEST_FL_OFFLINE) {
 		/* no tests are done online */
-		data[TSNEP_TEST_ENABLE] = 0;
-		data[TSNEP_TEST_TAPRIO] = 0;
-		data[TSNEP_TEST_TAPRIO_CHANGE] = 0;
-		data[TSNEP_TEST_TAPRIO_EXTENSION] = 0;
+		for (i = 0; i < count; i++)
+			data[i] = 0;
 
 		return;
 	}
@@ -808,4 +818,12 @@ void tsnep_ethtool_self_test(struct net_device *netdev,
 		eth_test->flags |= ETH_TEST_FL_FAILED;
 		data[TSNEP_TEST_TAPRIO_EXTENSION] = 1;
 	}
+	data += TSNEP_TEST_COUNT;
+
+	net_selftest_set(NET_SELFTEST_LOOPBACK_SPEED, 100, netdev, eth_test,
+			 data);
+	data += net_selftest_set_get_count(NET_SELFTEST_LOOPBACK_SPEED);
+
+	net_selftest_set(NET_SELFTEST_LOOPBACK_SPEED, 1000, netdev, eth_test,
+			 data);
 }
-- 
2.39.5