* [PATCH net-next v10 0/5] Support loopback mode speed selection
@ 2025-03-12 20:30 Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 1/5] net: phy: Allow loopback speed selection for PHY drivers Gerhard Engleder
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-12 20:30 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni
Cc: netdev, 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.
v10:
- remove selftests, because Anrew Lunn expects a new netlink API for
selftests and the selftest patches should wait for it
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 (5):
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
drivers/net/ethernet/engleder/tsnep_main.c | 21 +++--
.../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 ++--
net/core/selftests.c | 4 +-
16 files changed, 209 insertions(+), 107 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next v10 1/5] net: phy: Allow loopback speed selection for PHY drivers
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
@ 2025-03-12 20:30 ` Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 2/5] net: phy: Support speed selection for PHY loopback Gerhard Engleder
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-12 20:30 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni
Cc: netdev, 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 b2d32fbc8c85..b6828019c34c 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1837,9 +1837,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;
@@ -2608,12 +2608,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 c4a6385faf41..e8e42888f3e1 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1136,8 +1136,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 */
@@ -1930,7 +1938,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);
@@ -1972,7 +1980,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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next v10 2/5] net: phy: Support speed selection for PHY loopback
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 1/5] net: phy: Allow loopback speed selection for PHY drivers Gerhard Engleder
@ 2025-03-12 20:30 ` Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support Gerhard Engleder
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-12 20:30 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni
Cc: netdev, 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 b6828019c34c..6e1be6a6a4fc 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1817,41 +1817,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 e8e42888f3e1..260168b92ab3 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1823,7 +1823,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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 1/5] net: phy: Allow loopback speed selection for PHY drivers Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 2/5] net: phy: Support speed selection for PHY loopback Gerhard Engleder
@ 2025-03-12 20:30 ` Gerhard Engleder
2025-03-13 8:57 ` Maxime Chevallier
2025-03-12 20:30 ` [PATCH net-next v10 4/5] net: phy: marvell: Align set_loopback() implementation Gerhard Engleder
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-12 20:30 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni
Cc: netdev, 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 289e1d56aa65..24882d30f685 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -1032,6 +1032,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,
@@ -5565,6 +5588,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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next v10 4/5] net: phy: marvell: Align set_loopback() implementation
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
` (2 preceding siblings ...)
2025-03-12 20:30 ` [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support Gerhard Engleder
@ 2025-03-12 20:30 ` Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 5/5] tsnep: Select speed for loopback Gerhard Engleder
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-12 20:30 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni
Cc: netdev, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH net-next v10 5/5] tsnep: Select speed for loopback
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
` (3 preceding siblings ...)
2025-03-12 20:30 ` [PATCH net-next v10 4/5] net: phy: marvell: Align set_loopback() implementation Gerhard Engleder
@ 2025-03-12 20:30 ` Gerhard Engleder
2025-03-20 6:26 ` [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
2025-03-20 7:50 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-12 20:30 UTC (permalink / raw)
To: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni
Cc: netdev, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support
2025-03-12 20:30 ` [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support Gerhard Engleder
@ 2025-03-13 8:57 ` Maxime Chevallier
2025-03-13 9:35 ` Gerhard Engleder
0 siblings, 1 reply; 12+ messages in thread
From: Maxime Chevallier @ 2025-03-13 8:57 UTC (permalink / raw)
To: Gerhard Engleder
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev
Hello Gerhard,
On Wed, 12 Mar 2025 21:30:08 +0100
Gerhard Engleder <gerhard@engleder-embedded.com> wrote:
> 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 289e1d56aa65..24882d30f685 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -1032,6 +1032,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);
Maybe I don't fully get it, but it looks to me that you poll, waiting
for the link to become up. As you are in local loopback mode, how does
that work ? Do you need connection to an active LP for loopback to
work, or does the BMSR_LSTATUS bit behave differently under local
loopback ?
Maxime
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support
2025-03-13 8:57 ` Maxime Chevallier
@ 2025-03-13 9:35 ` Gerhard Engleder
2025-03-13 9:41 ` Maxime Chevallier
0 siblings, 1 reply; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-13 9:35 UTC (permalink / raw)
To: Maxime Chevallier
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev
Am 13.03.2025 09:57, schrieb Maxime Chevallier:
> Hello Gerhard,
>
> On Wed, 12 Mar 2025 21:30:08 +0100
> Gerhard Engleder <gerhard@engleder-embedded.com> wrote:
>
>> 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 289e1d56aa65..24882d30f685 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -1032,6 +1032,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);
>
> Maybe I don't fully get it, but it looks to me that you poll, waiting
> for the link to become up. As you are in local loopback mode, how does
> that work ? Do you need connection to an active LP for loopback to
> work, or does the BMSR_LSTATUS bit behave differently under local
> loopback ?
Yes I'm polling for link to come up. This is identical to
genphy_loopback().
There is no need for a link partner to get a link up in loopback mode.
BMSR_LSTATUS reflects the loopback as link in this case.
This series allows to configure a loopback with defined speed without
any
link partner. Currently for PHY loopback a link partner is needed in
some
cases (starting with 1 Gbps loopback).
Gerhard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support
2025-03-13 9:35 ` Gerhard Engleder
@ 2025-03-13 9:41 ` Maxime Chevallier
0 siblings, 0 replies; 12+ messages in thread
From: Maxime Chevallier @ 2025-03-13 9:41 UTC (permalink / raw)
To: Gerhard Engleder
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev
Hi,
On Thu, 13 Mar 2025 10:35:47 +0100
Gerhard Engleder <gerhard@engleder-embedded.com> wrote:
> Am 13.03.2025 09:57, schrieb Maxime Chevallier:
> > Hello Gerhard,
> >
> > On Wed, 12 Mar 2025 21:30:08 +0100
> > Gerhard Engleder <gerhard@engleder-embedded.com> wrote:
> >
> >> 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 289e1d56aa65..24882d30f685 100644
> >> --- a/drivers/net/phy/micrel.c
> >> +++ b/drivers/net/phy/micrel.c
> >> @@ -1032,6 +1032,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);
> >
> > Maybe I don't fully get it, but it looks to me that you poll, waiting
> > for the link to become up. As you are in local loopback mode, how does
> > that work ? Do you need connection to an active LP for loopback to
> > work, or does the BMSR_LSTATUS bit behave differently under local
> > loopback ?
>
> Yes I'm polling for link to come up. This is identical to
> genphy_loopback().
> There is no need for a link partner to get a link up in loopback mode.
> BMSR_LSTATUS reflects the loopback as link in this case.
>
> This series allows to configure a loopback with defined speed without
> any
> link partner. Currently for PHY loopback a link partner is needed in
> some
> cases (starting with 1 Gbps loopback).
Ok thanks a lot for the clarification ! It looks good to me in that
case :)
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v10 0/5] Support loopback mode speed selection
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
` (4 preceding siblings ...)
2025-03-12 20:30 ` [PATCH net-next v10 5/5] tsnep: Select speed for loopback Gerhard Engleder
@ 2025-03-20 6:26 ` Gerhard Engleder
2025-03-20 7:32 ` Paolo Abeni
2025-03-20 7:50 ` patchwork-bot+netdevbpf
6 siblings, 1 reply; 12+ messages in thread
From: Gerhard Engleder @ 2025-03-20 6:26 UTC (permalink / raw)
To: andrew
Cc: netdev, hkallweit1, linux, davem, edumazet, kuba, pabeni, ltrager,
Jijie Shao
On 12.03.25 21:30, Gerhard Engleder wrote:
> 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.
>
> v10:
> - remove selftests, because Anrew Lunn expects a new netlink API for
> selftests and the selftest patches should wait for it
>
Hello Andrew,
The patchset now does not touch any selftest code anymore. It now only
fixes the 1Gbps loopback, which requires a link partner since
6ff3cddc365b. tsnep is using the extended phy_loopback() interface
to select the loopback speed. Also the phy_loopback() usage in tsnep
could be simplified, because thanks to your review comments link speed
changes are now signaled correctly by phy_loopback().
I'm curious about the work of Lee Trager and I will definitely take a
look on it. I will take a look to the netdev 0x19 talk as soon as the
slides or the recording is available.
I did not get an answer from you to my last reply of v9. That's the
reason why I decided to post v10 without selftests. How to proceed
with this changes? The development cycle is near the end, so maybe
you want to delay this change to the beginning of the next development
cycle?
Gerhard
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v10 0/5] Support loopback mode speed selection
2025-03-20 6:26 ` [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
@ 2025-03-20 7:32 ` Paolo Abeni
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Abeni @ 2025-03-20 7:32 UTC (permalink / raw)
To: Gerhard Engleder, andrew
Cc: netdev, hkallweit1, linux, davem, edumazet, kuba, ltrager,
Jijie Shao
On 3/20/25 7:26 AM, Gerhard Engleder wrote:
> On 12.03.25 21:30, Gerhard Engleder wrote:
>> 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.
>>
>> v10:
>> - remove selftests, because Anrew Lunn expects a new netlink API for
>> selftests and the selftest patches should wait for it
>>
>
> Hello Andrew,
>
> The patchset now does not touch any selftest code anymore. It now only
> fixes the 1Gbps loopback, which requires a link partner since
> 6ff3cddc365b. tsnep is using the extended phy_loopback() interface
> to select the loopback speed. Also the phy_loopback() usage in tsnep
> could be simplified, because thanks to your review comments link speed
> changes are now signaled correctly by phy_loopback().
>
> I'm curious about the work of Lee Trager and I will definitely take a
> look on it. I will take a look to the netdev 0x19 talk as soon as the
> slides or the recording is available.
>
> I did not get an answer from you to my last reply of v9. That's the
> reason why I decided to post v10 without selftests. How to proceed
> with this changes? The development cycle is near the end, so maybe
> you want to delay this change to the beginning of the next development
> cycle?
FTR, I reached for Andrew off-list and he is ok with v10. Given the
large PW backlog and the upcoming merge window, I'm going to apply the
patches possibly before his explicit ack on the ML.
/P
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next v10 0/5] Support loopback mode speed selection
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
` (5 preceding siblings ...)
2025-03-20 6:26 ` [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
@ 2025-03-20 7:50 ` patchwork-bot+netdevbpf
6 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-03-20 7:50 UTC (permalink / raw)
To: Gerhard Engleder
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Wed, 12 Mar 2025 21:30:05 +0100 you wrote:
> 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.
>
> [...]
Here is the summary with links:
- [net-next,v10,1/5] net: phy: Allow loopback speed selection for PHY drivers
https://git.kernel.org/netdev/net-next/c/45456e38c44e
- [net-next,v10,2/5] net: phy: Support speed selection for PHY loopback
https://git.kernel.org/netdev/net-next/c/0d60fd50328a
- [net-next,v10,3/5] net: phy: micrel: Add loopback support
https://git.kernel.org/netdev/net-next/c/fe4bf60ffdff
- [net-next,v10,4/5] net: phy: marvell: Align set_loopback() implementation
https://git.kernel.org/netdev/net-next/c/1a0df6c96ce5
- [net-next,v10,5/5] tsnep: Select speed for loopback
https://git.kernel.org/netdev/net-next/c/163d744d020e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-03-20 7:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-12 20:30 [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 1/5] net: phy: Allow loopback speed selection for PHY drivers Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 2/5] net: phy: Support speed selection for PHY loopback Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 3/5] net: phy: micrel: Add loopback support Gerhard Engleder
2025-03-13 8:57 ` Maxime Chevallier
2025-03-13 9:35 ` Gerhard Engleder
2025-03-13 9:41 ` Maxime Chevallier
2025-03-12 20:30 ` [PATCH net-next v10 4/5] net: phy: marvell: Align set_loopback() implementation Gerhard Engleder
2025-03-12 20:30 ` [PATCH net-next v10 5/5] tsnep: Select speed for loopback Gerhard Engleder
2025-03-20 6:26 ` [PATCH net-next v10 0/5] Support loopback mode speed selection Gerhard Engleder
2025-03-20 7:32 ` Paolo Abeni
2025-03-20 7:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox