[PATCH net-next 0/2] net: stmmac: only use MAC loopback for selftests

[PATCH net-next 0/2] net: stmmac: only use MAC loopback for selftests

[Maxime Chevallier]

stmmac selftests currently use PHY loopback when a PHY is attached, and
fallback to MAC loopback otherwise. PHY loopback however isn't ideal nor
necessary for the tests we are running, that only stress the internal
stmmac features.

Some PHYs bring the carrier state down when in loopback mode, which will
prevent any packet transmission even for ourselves, making selftests
fail for non-stmmac related reasons.

Let's rely only on MAC-side loopback for selftests, making it clear that
any problem found with stmmac selftests are indeed caused by the stmmac
driver, and not external factors.

This was tested on :

 - Cyclone V with RGMII link to KSZ9031
 - Cyclone V with 1000BaseX
 - imx8mp with RGMII link to KSZ9131
 - stm32mp157& with RGMII link to RTL8211F
 - Allwinner H2+ with an internal PHY

Maxime Chevallier (2):
  net: stmmac: Don't use PHY loopback for selftests
  net: stmmac: Don't rely on the PHY for flow-control testing

 .../stmicro/stmmac/stmmac_selftests.c         | 117 ++----------------
 1 file changed, 11 insertions(+), 106 deletions(-)

-- 
2.55.0

[PATCH net-next 1/2] net: stmmac: Don't use PHY loopback for selftests

[Maxime Chevallier]

Stmmac selftests validate the internal behaviour of the various IPs,
using local loopback. The current logic is relies on PHY-side local
loopback if a PHY is attached, with a fallback to MAC loopback
otherwise.

However, PHY loopback is currently fragile especially for stmmac that
may require RXC to be provided from the PHY. Some PHYs shutdown RXC
while in loopback, while others will report carrier off when in local
loopback. This also fails when using SFP setup with a module that embeds
a PHY, that may also fail to enter loopback.

MAC loopback is done at the GMII level on dwmac, allowing the internal
to be just as meaningful as PHY-loopback testing.

Let's simplify stmmac selftests by only relying on MAC-side local
loopback, which makes the selftests runnable on a wider HW variety.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 .../stmicro/stmmac/stmmac_selftests.c         | 112 ++----------------
 1 file changed, 9 insertions(+), 103 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index 29e824bd90ca..c0e5dee86451 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -374,25 +374,6 @@ static int stmmac_test_mac_loopback(struct stmmac_priv *priv)
 	return __stmmac_test_loopback(priv, &attr);
 }
 
-static int stmmac_test_phy_loopback(struct stmmac_priv *priv)
-{
-	struct stmmac_packet_attrs attr = { };
-	int ret;
-
-	if (!priv->dev->phydev)
-		return -EOPNOTSUPP;
-
-	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, 0);
-	return ret;
-}
-
 static int stmmac_test_mmc(struct stmmac_priv *priv)
 {
 	struct stmmac_counters initial, final;
@@ -1815,10 +1796,6 @@ static int stmmac_test_tbs(struct stmmac_priv *priv)
 	return ret;
 }
 
-#define STMMAC_LOOPBACK_NONE	0
-#define STMMAC_LOOPBACK_MAC	1
-#define STMMAC_LOOPBACK_PHY	2
-
 static const struct stmmac_test {
 	char name[ETH_GSTRING_LEN];
 	int lb;
@@ -1826,131 +1803,96 @@ static const struct stmmac_test {
 } stmmac_selftests[] = {
 	{
 		.name = "MAC Loopback               ",
-		.lb = STMMAC_LOOPBACK_MAC,
 		.fn = stmmac_test_mac_loopback,
-	}, {
-		.name = "PHY Loopback               ",
-		.lb = STMMAC_LOOPBACK_NONE, /* Test will handle it */
-		.fn = stmmac_test_phy_loopback,
 	}, {
 		.name = "MMC Counters               ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_mmc,
 	}, {
 		.name = "EEE                        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_eee,
 	}, {
 		.name = "Hash Filter MC             ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_hfilt,
 	}, {
 		.name = "Perfect Filter UC          ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_pfilt,
 	}, {
 		.name = "MC Filter                  ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_mcfilt,
 	}, {
 		.name = "UC Filter                  ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_ucfilt,
 	}, {
 		.name = "Flow Control               ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_flowctrl,
 	}, {
 		.name = "RSS                        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_rss,
 	}, {
 		.name = "VLAN Filtering             ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_vlanfilt,
 	}, {
 		.name = "VLAN Filtering (perf)      ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_vlanfilt_perfect,
 	}, {
 		.name = "Double VLAN Filter         ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_dvlanfilt,
 	}, {
 		.name = "Double VLAN Filter (perf)  ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_dvlanfilt_perfect,
 	}, {
 		.name = "Flexible RX Parser         ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_rxp,
 	}, {
 		.name = "SA Insertion (desc)        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_desc_sai,
 	}, {
 		.name = "SA Replacement (desc)      ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_desc_sar,
 	}, {
 		.name = "SA Insertion (reg)         ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_reg_sai,
 	}, {
 		.name = "SA Replacement (reg)       ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_reg_sar,
 	}, {
 		.name = "VLAN TX Insertion          ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_vlanoff,
 	}, {
 		.name = "SVLAN TX Insertion         ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_svlanoff,
 	}, {
 		.name = "L3 DA Filtering            ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_l3filt_da,
 	}, {
 		.name = "L3 SA Filtering            ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_l3filt_sa,
 	}, {
 		.name = "L4 DA TCP Filtering        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_l4filt_da_tcp,
 	}, {
 		.name = "L4 SA TCP Filtering        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_l4filt_sa_tcp,
 	}, {
 		.name = "L4 DA UDP Filtering        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_l4filt_da_udp,
 	}, {
 		.name = "L4 SA UDP Filtering        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_l4filt_sa_udp,
 	}, {
 		.name = "ARP Offload                ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_arpoffload,
 	}, {
 		.name = "Jumbo Frame                ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_jumbo,
 	}, {
 		.name = "Multichannel Jumbo         ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_mjumbo,
 	}, {
 		.name = "Split Header               ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_sph,
 	}, {
 		.name = "TBS (ETF Scheduler)        ",
-		.lb = STMMAC_LOOPBACK_PHY,
 		.fn = stmmac_test_tbs,
 	},
 };
@@ -1978,57 +1920,21 @@ void stmmac_selftest_run(struct net_device *dev,
 	/* Wait for queues drain */
 	msleep(200);
 
-	for (i = 0; i < count; i++) {
-		ret = 0;
-
-		switch (stmmac_selftests[i].lb) {
-		case STMMAC_LOOPBACK_PHY:
-			ret = -EOPNOTSUPP;
-			if (dev->phydev)
-				ret = phy_loopback(dev->phydev, true, 0);
-			if (!ret)
-				break;
-			fallthrough;
-		case STMMAC_LOOPBACK_MAC:
-			ret = stmmac_set_mac_loopback(priv, priv->ioaddr, true);
-			break;
-		case STMMAC_LOOPBACK_NONE:
-			break;
-		default:
-			ret = -EOPNOTSUPP;
-			break;
-		}
-
-		/*
-		 * First tests will always be MAC / PHY loopback. If any of
-		 * them is not supported we abort earlier.
-		 */
-		if (ret) {
-			netdev_err(priv->dev, "Loopback is not supported\n");
-			etest->flags |= ETH_TEST_FL_FAILED;
-			break;
-		}
+	ret = stmmac_set_mac_loopback(priv, priv->ioaddr, true);
+	if (ret) {
+		netdev_err(priv->dev, "Loopback is not supported\n");
+		etest->flags |= ETH_TEST_FL_FAILED;
+		return;
+	}
 
+	for (i = 0; i < count; i++) {
 		ret = stmmac_selftests[i].fn(priv);
 		if (ret && (ret != -EOPNOTSUPP))
 			etest->flags |= ETH_TEST_FL_FAILED;
 		buf[i] = ret;
-
-		switch (stmmac_selftests[i].lb) {
-		case STMMAC_LOOPBACK_PHY:
-			ret = -EOPNOTSUPP;
-			if (dev->phydev)
-				ret = phy_loopback(dev->phydev, false, 0);
-			if (!ret)
-				break;
-			fallthrough;
-		case STMMAC_LOOPBACK_MAC:
-			stmmac_set_mac_loopback(priv, priv->ioaddr, false);
-			break;
-		default:
-			break;
-		}
 	}
+
+	stmmac_set_mac_loopback(priv, priv->ioaddr, false);
 }
 
 void stmmac_selftest_get_strings(struct stmmac_priv *priv, u8 *data)
-- 
2.55.0

[PATCH net-next 2/2] net: stmmac: Don't rely on the PHY for flow-control testing

[Maxime Chevallier]

For flow-control testing in loopback mode, we don't need to ask what the
PHY is currently using as pause/asym settings. The PHY is no longer
involved in selftest, we rely strictly on MAC loopback. We therefore
only need to know if the MAC supports Symmetric pause for the test, as
we exercise both TX and RX pause support in the selftest.

Remove phydev requirement for flowcontrol selftest as well as the
AsymPause requirement.

With that, we can also drop the linux/phy.h include.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
index c0e5dee86451..1df26c217f9a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -11,7 +11,6 @@
 #include <linux/crc32.h>
 #include <linux/ethtool.h>
 #include <linux/ip.h>
-#include <linux/phy.h>
 #include <linux/udp.h>
 #include <net/pkt_cls.h>
 #include <net/pkt_sched.h>
@@ -716,13 +715,13 @@ static int stmmac_test_flowctrl_validate(struct sk_buff *skb,
 static int stmmac_test_flowctrl(struct stmmac_priv *priv)
 {
 	unsigned char paddr[ETH_ALEN] = {0x01, 0x80, 0xC2, 0x00, 0x00, 0x01};
-	struct phy_device *phydev = priv->dev->phydev;
 	u32 rx_cnt = priv->plat->rx_queues_to_use;
+	struct mac_device_info *mac = priv->hw;
 	struct stmmac_test_priv *tpriv;
 	unsigned int pkt_count;
 	int i, ret = 0;
 
-	if (!phydev || (!phydev->pause && !phydev->asym_pause))
+	if (!(mac->link.caps & MAC_SYM_PAUSE))
 		return -EOPNOTSUPP;
 
 	tpriv = kzalloc_obj(*tpriv);
-- 
2.55.0

Re: [PATCH net-next 2/2] net: stmmac: Don't rely on the PHY for flow-control testing

[Oleksij Rempel]

On Tue, Jul 28, 2026 at 05:57:27PM +0200, Maxime Chevallier wrote:
> For flow-control testing in loopback mode, we don't need to ask what the
> PHY is currently using as pause/asym settings. The PHY is no longer
> involved in selftest, we rely strictly on MAC loopback. We therefore

In case of the Full-Duplex, Link-wide PAUSE (IEEE 802.3 Annex 31B),
the PHY is not involved in actual flow control. PHY role is limited only
to autoneg part - announce own and receive link partners caps.

With this reasoning this patch will have more weight, i guess :)

> only need to know if the MAC supports Symmetric pause for the test, as
> we exercise both TX and RX pause support in the selftest.
> 
> Remove phydev requirement for flowcontrol selftest as well as the
> AsymPause requirement.

This is currently the weak point of generic and stmmac selftests,
we actually need to force the link in some specific state.

On the PHY side we would need to enabled force link mode (disable
autoneg) and bypass link-up status (PHY may still signal link down)

> With that, we can also drop the linux/phy.h include.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Otherwise, LGTM:
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>

Thank you!

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next 1/2] net: stmmac: Don't use PHY loopback for selftests

[Oleksij Rempel]

On Tue, Jul 28, 2026 at 05:57:26PM +0200, Maxime Chevallier wrote:
> Stmmac selftests validate the internal behaviour of the various IPs,
> using local loopback. The current logic is relies on PHY-side local
> loopback if a PHY is attached, with a fallback to MAC loopback
> otherwise.
> 
> However, PHY loopback is currently fragile especially for stmmac that
> may require RXC to be provided from the PHY. Some PHYs shutdown RXC
> while in loopback, while others will report carrier off when in local
> loopback. This also fails when using SFP setup with a module that embeds
> a PHY, that may also fail to enter loopback.
> 
> MAC loopback is done at the GMII level on dwmac, allowing the internal
> to be just as meaningful as PHY-loopback testing.
> 
> Let's simplify stmmac selftests by only relying on MAC-side local
> loopback, which makes the selftests runnable on a wider HW variety.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>  .../stmicro/stmmac/stmmac_selftests.c         | 112 ++----------------
>  1 file changed, 9 insertions(+), 103 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> index 29e824bd90ca..c0e5dee86451 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
> @@ -374,25 +374,6 @@ static int stmmac_test_mac_loopback(struct stmmac_priv *priv)
>  	return __stmmac_test_loopback(priv, &attr);
>  }
>  
> -static int stmmac_test_phy_loopback(struct stmmac_priv *priv)
> -{
> -	struct stmmac_packet_attrs attr = { };
> -	int ret;
> -
> -	if (!priv->dev->phydev)
> -		return -EOPNOTSUPP;
> -
> -	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, 0);
> -	return ret;
> -}
> -
>  static int stmmac_test_mmc(struct stmmac_priv *priv)
>  {
>  	struct stmmac_counters initial, final;
> @@ -1815,10 +1796,6 @@ static int stmmac_test_tbs(struct stmmac_priv *priv)
>  	return ret;
>  }
>  
> -#define STMMAC_LOOPBACK_NONE	0
> -#define STMMAC_LOOPBACK_MAC	1
> -#define STMMAC_LOOPBACK_PHY	2
> -
>  static const struct stmmac_test {
>  	char name[ETH_GSTRING_LEN];
>  	int lb;
> @@ -1826,131 +1803,96 @@ static const struct stmmac_test {
>  } stmmac_selftests[] = {
>  	{
>  		.name = "MAC Loopback               ",
> -		.lb = STMMAC_LOOPBACK_MAC,
>  		.fn = stmmac_test_mac_loopback,
> -	}, {
> -		.name = "PHY Loopback               ",
> -		.lb = STMMAC_LOOPBACK_NONE, /* Test will handle it */
> -		.fn = stmmac_test_phy_loopback,
>  	}, {
>  		.name = "MMC Counters               ",
> -		.lb = STMMAC_LOOPBACK_PHY,
>  		.fn = stmmac_test_mmc,
>  	}, {
>  		.name = "EEE                        ",
> -		.lb = STMMAC_LOOPBACK_PHY,
>  		.fn = stmmac_test_eee,

Hm, not sure this test can be properly executed in all conditions.
We do not support forced LPI mode from phylink perspective, and it is
not forced by the tests. If it passes, only by accident if link partner
local system and link partner advertise the EEE support. 

>  	}, {
>  		.name = "Hash Filter MC             ",
> -		.lb = STMMAC_LOOPBACK_PHY,
>  		.fn = stmmac_test_hfilt,
>  	}, {
>  		.name = "Perfect Filter UC          ",
> -		.lb = STMMAC_LOOPBACK_PHY,
>  		.fn = stmmac_test_pfilt,
>  	}, {
>  		.name = "MC Filter                  ",
> -		.lb = STMMAC_LOOPBACK_PHY,
>  		.fn = stmmac_test_mcfilt,
>  	}, {
>  		.name = "UC Filter                  ",
> -		.lb = STMMAC_LOOPBACK_PHY,
>  		.fn = stmmac_test_ucfilt,
>  	}, {
>  		.name = "Flow Control               ",
> -		.lb = STMMAC_LOOPBACK_PHY,
>  		.fn = stmmac_test_flowctrl,

Interesting test, it will be cool to have it in generic form. Probably
most of them, but it will be a lot of work.

So, yes - most of this tests do not relay on the PHY, instead - on some
kind of running link configuration. But this tests seems to not enforce
needed link configuration before and after patches - correct?

Best Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |