[PATCH net-next v3 0/6] provide generic net selftest support

[PATCH net-next v3 0/6] provide generic net selftest support

[Oleksij Rempel]

changes v3:
- make more granular tests
- enable loopback for all PHYs by default
- fix allmodconfig build errors
- poll for link status update after switching to the loopback mode

changes v2:
- make generic selftests available for all networking devices.
- make use of net_selftest* on FEC, ag71xx and all DSA switches.
- add loopback support on more PHYs.

This patch set provides diagnostic capabilities for some iMX, ag71xx or
any DSA based devices. For proper functionality, PHY loopback support is
needed.
So far there is only initial infrastructure with basic tests. 

Oleksij Rempel (6):
  net: phy: execute genphy_loopback() per default on all PHYs
  net: phy: genphy_loopback: add link speed configuration
  net: add generic selftest support
  net: fec: make use of generic NET_SELFTESTS library
  net: ag71xx: make use of generic NET_SELFTESTS library
  net: dsa: enable selftest support for all switches by default

 drivers/net/ethernet/atheros/Kconfig      |   1 +
 drivers/net/ethernet/atheros/ag71xx.c     |  20 +-
 drivers/net/ethernet/freescale/Kconfig    |   1 +
 drivers/net/ethernet/freescale/fec_main.c |   7 +
 drivers/net/phy/phy.c                     |   3 +-
 drivers/net/phy/phy_device.c              |  35 +-
 include/linux/phy.h                       |   1 +
 include/net/dsa.h                         |   2 +
 include/net/selftests.h                   |  12 +
 net/Kconfig                               |   4 +
 net/core/Makefile                         |   1 +
 net/core/selftests.c                      | 400 ++++++++++++++++++++++
 net/dsa/Kconfig                           |   1 +
 net/dsa/slave.c                           |  21 ++
 14 files changed, 500 insertions(+), 9 deletions(-)
 create mode 100644 include/net/selftests.h
 create mode 100644 net/core/selftests.c

-- 
2.29.2

[PATCH net-next v3 1/6] net: phy: execute genphy_loopback() per default on all PHYs

[Oleksij Rempel]

The generic loopback is really generic and is defined by the 802.3
standard, we should just mandate that drivers implement a custom
loopback if the generic one cannot work.

Suggested-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy_device.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 73d29fd5e03d..320a3e5cd10a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1777,6 +1777,9 @@ int phy_loopback(struct phy_device *phydev, bool enable)
 	struct phy_driver *phydrv = to_phy_driver(phydev->mdio.dev.driver);
 	int ret = 0;
 
+	if (!phydrv)
+		return -ENODEV;
+
 	mutex_lock(&phydev->lock);
 
 	if (enable && phydev->loopback_enabled) {
@@ -1789,10 +1792,10 @@ int phy_loopback(struct phy_device *phydev, bool enable)
 		goto out;
 	}
 
-	if (phydev->drv && phydrv->set_loopback)
+	if (phydrv->set_loopback)
 		ret = phydrv->set_loopback(phydev, enable);
 	else
-		ret = -EOPNOTSUPP;
+		ret = genphy_loopback(phydev, enable);
 
 	if (ret)
 		goto out;
-- 
2.29.2

[PATCH net-next v3 2/6] net: phy: genphy_loopback: add link speed configuration

[Oleksij Rempel]

In case of loopback, in most cases we need to disable autoneg support
and force some speed configuration. Otherwise, depending on currently
active auto negotiated link speed, the loopback may or may not work.

This patch was tested with following PHYs: TJA1102, KSZ8081, KSZ9031,
AT8035, AR9331.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy.c        |  3 ++-
 drivers/net/phy/phy_device.c | 28 ++++++++++++++++++++++++++--
 include/linux/phy.h          |  1 +
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index fc2e7cb5b2e5..1f0512e39c65 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -701,7 +701,7 @@ int phy_start_cable_test_tdr(struct phy_device *phydev,
 }
 EXPORT_SYMBOL(phy_start_cable_test_tdr);
 
-static int phy_config_aneg(struct phy_device *phydev)
+int phy_config_aneg(struct phy_device *phydev)
 {
 	if (phydev->drv->config_aneg)
 		return phydev->drv->config_aneg(phydev);
@@ -714,6 +714,7 @@ static int phy_config_aneg(struct phy_device *phydev)
 
 	return genphy_config_aneg(phydev);
 }
+EXPORT_SYMBOL(phy_config_aneg);
 
 /**
  * phy_check_link_status - check link status and set state accordingly
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 320a3e5cd10a..0a2d8bedf73d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -2565,8 +2565,32 @@ EXPORT_SYMBOL(genphy_resume);
 
 int genphy_loopback(struct phy_device *phydev, bool enable)
 {
-	return phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK,
-			  enable ? BMCR_LOOPBACK : 0);
+	if (enable) {
+		u16 val, ctl = BMCR_LOOPBACK;
+		int ret;
+
+		if (phydev->speed == SPEED_1000)
+			ctl |= BMCR_SPEED1000;
+		else if (phydev->speed == SPEED_100)
+			ctl |= BMCR_SPEED100;
+
+		if (phydev->duplex == DUPLEX_FULL)
+			ctl |= BMCR_FULLDPLX;
+
+		phy_modify(phydev, MII_BMCR, ~0, ctl);
+
+		ret = phy_read_poll_timeout(phydev, MII_BMSR, val,
+					    val & BMSR_LSTATUS,
+				    5000, 500000, true);
+		if (ret)
+			return ret;
+	} else {
+		phy_modify(phydev, MII_BMCR, BMCR_LOOPBACK, 0);
+
+		phy_config_aneg(phydev);
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL(genphy_loopback);
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 98fb441dd72e..98e351bb0964 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -1410,6 +1410,7 @@ void phy_disconnect(struct phy_device *phydev);
 void phy_detach(struct phy_device *phydev);
 void phy_start(struct phy_device *phydev);
 void phy_stop(struct phy_device *phydev);
+int phy_config_aneg(struct phy_device *phydev);
 int phy_start_aneg(struct phy_device *phydev);
 int phy_aneg_done(struct phy_device *phydev);
 int phy_speed_down(struct phy_device *phydev, bool sync);
-- 
2.29.2

[PATCH net-next v3 3/6] net: add generic selftest support

[Oleksij Rempel]

Port some parts of the stmmac selftest and reuse it as basic generic selftest
library. This patch was tested with following combinations:
- iMX6DL FEC -> AT8035
- iMX6DL FEC -> SJA1105Q switch -> KSZ8081
- iMX6DL FEC -> SJA1105Q switch -> KSZ9031
- AR9331 ag71xx -> AR9331 PHY
- AR9331 ag71xx -> AR9331 switch -> AR9331 PHY

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/net/selftests.h |  12 ++
 net/Kconfig             |   4 +
 net/core/Makefile       |   1 +
 net/core/selftests.c    | 400 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 417 insertions(+)
 create mode 100644 include/net/selftests.h
 create mode 100644 net/core/selftests.c

diff --git a/include/net/selftests.h b/include/net/selftests.h
new file mode 100644
index 000000000000..9993b9498cf3
--- /dev/null
+++ b/include/net/selftests.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _NET_SELFTESTS
+#define _NET_SELFTESTS
+
+#include <linux/ethtool.h>
+
+void net_selftest(struct net_device *ndev, struct ethtool_test *etest,
+		  u64 *buf);
+int net_selftest_get_count(void);
+void net_selftest_get_strings(u8 *data);
+
+#endif /* _NET_SELFTESTS */
diff --git a/net/Kconfig b/net/Kconfig
index 9c456acc379e..8d955195c069 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -429,6 +429,10 @@ config GRO_CELLS
 config SOCK_VALIDATE_XMIT
 	bool
 
+config NET_SELFTESTS
+	def_tristate PHYLIB
+	depends on PHYLIB
+
 config NET_SOCK_MSG
 	bool
 	default n
diff --git a/net/core/Makefile b/net/core/Makefile
index 0c2233c826fd..1a6168d8f23b 100644
--- a/net/core/Makefile
+++ b/net/core/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_NET_DEVLINK) += devlink.o
 obj-$(CONFIG_GRO_CELLS) += gro_cells.o
 obj-$(CONFIG_FAILOVER) += failover.o
 ifeq ($(CONFIG_INET),y)
+obj-$(CONFIG_NET_SELFTESTS) += selftests.o
 obj-$(CONFIG_NET_SOCK_MSG) += skmsg.o
 obj-$(CONFIG_BPF_SYSCALL) += sock_map.o
 endif
diff --git a/net/core/selftests.c b/net/core/selftests.c
new file mode 100644
index 000000000000..ba7b0171974c
--- /dev/null
+++ b/net/core/selftests.c
@@ -0,0 +1,400 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
+ * stmmac Selftests Support
+ *
+ * Author: Jose Abreu <joabreu@synopsys.com>
+ *
+ * Ported from stmmac by:
+ * Copyright (C) 2021 Oleksij Rempel <o.rempel@pengutronix.de>
+ */
+
+#include <linux/phy.h>
+#include <net/selftests.h>
+#include <net/tcp.h>
+#include <net/udp.h>
+
+struct net_packet_attrs {
+	unsigned char *src;
+	unsigned char *dst;
+	u32 ip_src;
+	u32 ip_dst;
+	bool tcp;
+	u16 sport;
+	u16 dport;
+	int timeout;
+	int size;
+	int max_size;
+	u8 id;
+	u16 queue_mapping;
+};
+
+struct net_test_priv {
+	struct net_packet_attrs *packet;
+	struct packet_type pt;
+	struct completion comp;
+	int double_vlan;
+	int vlan_id;
+	int ok;
+};
+
+struct netsfhdr {
+	__be32 version;
+	__be64 magic;
+	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,
+					struct net_packet_attrs *attr)
+{
+	struct sk_buff *skb = NULL;
+	struct udphdr *uhdr = NULL;
+	struct tcphdr *thdr = NULL;
+	struct netsfhdr *shdr;
+	struct ethhdr *ehdr;
+	struct iphdr *ihdr;
+	int iplen, size;
+
+	size = attr->size + NET_TEST_PKT_SIZE;
+
+	if (attr->tcp)
+		size += sizeof(struct tcphdr);
+	else
+		size += sizeof(struct udphdr);
+
+	if (attr->max_size && attr->max_size > size)
+		size = attr->max_size;
+
+	skb = netdev_alloc_skb(ndev, size);
+	if (!skb)
+		return NULL;
+
+	prefetchw(skb->data);
+
+	ehdr = skb_push(skb, ETH_HLEN);
+	skb_reset_mac_header(skb);
+
+	skb_set_network_header(skb, skb->len);
+	ihdr = skb_put(skb, sizeof(*ihdr));
+
+	skb_set_transport_header(skb, skb->len);
+	if (attr->tcp)
+		thdr = skb_put(skb, sizeof(*thdr));
+	else
+		uhdr = skb_put(skb, sizeof(*uhdr));
+
+	eth_zero_addr(ehdr->h_dest);
+
+	if (attr->src)
+		ether_addr_copy(ehdr->h_source, attr->src);
+	if (attr->dst)
+		ether_addr_copy(ehdr->h_dest, attr->dst);
+
+	ehdr->h_proto = htons(ETH_P_IP);
+
+	if (attr->tcp) {
+		thdr->source = htons(attr->sport);
+		thdr->dest = htons(attr->dport);
+		thdr->doff = sizeof(struct tcphdr) / 4;
+		thdr->check = 0;
+	} else {
+		uhdr->source = htons(attr->sport);
+		uhdr->dest = htons(attr->dport);
+		uhdr->len = htons(sizeof(*shdr) + sizeof(*uhdr) + attr->size);
+		if (attr->max_size)
+			uhdr->len = htons(attr->max_size -
+					  (sizeof(*ihdr) + sizeof(*ehdr)));
+		uhdr->check = 0;
+	}
+
+	ihdr->ihl = 5;
+	ihdr->ttl = 32;
+	ihdr->version = 4;
+	if (attr->tcp)
+		ihdr->protocol = IPPROTO_TCP;
+	else
+		ihdr->protocol = IPPROTO_UDP;
+	iplen = sizeof(*ihdr) + sizeof(*shdr) + attr->size;
+	if (attr->tcp)
+		iplen += sizeof(*thdr);
+	else
+		iplen += sizeof(*uhdr);
+
+	if (attr->max_size)
+		iplen = attr->max_size - sizeof(*ehdr);
+
+	ihdr->tot_len = htons(iplen);
+	ihdr->frag_off = 0;
+	ihdr->saddr = htonl(attr->ip_src);
+	ihdr->daddr = htonl(attr->ip_dst);
+	ihdr->tos = 0;
+	ihdr->id = 0;
+	ip_send_check(ihdr);
+
+	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++;
+
+	if (attr->size)
+		skb_put(skb, attr->size);
+	if (attr->max_size && attr->max_size > skb->len)
+		skb_put(skb, attr->max_size - skb->len);
+
+	skb->csum = 0;
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	if (attr->tcp) {
+		thdr->check = ~tcp_v4_check(skb->len, ihdr->saddr,
+					    ihdr->daddr, 0);
+		skb->csum_start = skb_transport_header(skb) - skb->head;
+		skb->csum_offset = offsetof(struct tcphdr, check);
+	} else {
+		udp4_hwcsum(skb, ihdr->saddr, ihdr->daddr);
+	}
+
+	skb->protocol = htons(ETH_P_IP);
+	skb->pkt_type = PACKET_HOST;
+	skb->dev = ndev;
+
+	return skb;
+}
+
+static int net_test_loopback_validate(struct sk_buff *skb,
+				      struct net_device *ndev,
+				      struct packet_type *pt,
+				      struct net_device *orig_ndev)
+{
+	struct net_test_priv *tpriv = pt->af_packet_priv;
+	unsigned char *src = tpriv->packet->src;
+	unsigned char *dst = tpriv->packet->dst;
+	struct netsfhdr *shdr;
+	struct ethhdr *ehdr;
+	struct udphdr *uhdr;
+	struct tcphdr *thdr;
+	struct iphdr *ihdr;
+
+	skb = skb_unshare(skb, GFP_ATOMIC);
+	if (!skb)
+		goto out;
+
+	if (skb_linearize(skb))
+		goto out;
+	if (skb_headlen(skb) < (NET_TEST_PKT_SIZE - ETH_HLEN))
+		goto out;
+
+	ehdr = (struct ethhdr *)skb_mac_header(skb);
+	if (dst) {
+		if (!ether_addr_equal_unaligned(ehdr->h_dest, dst))
+			goto out;
+	}
+
+	if (src) {
+		if (!ether_addr_equal_unaligned(ehdr->h_source, src))
+			goto out;
+	}
+
+	ihdr = ip_hdr(skb);
+	if (tpriv->double_vlan)
+		ihdr = (struct iphdr *)(skb_network_header(skb) + 4);
+
+	if (tpriv->packet->tcp) {
+		if (ihdr->protocol != IPPROTO_TCP)
+			goto out;
+
+		thdr = (struct tcphdr *)((u8 *)ihdr + 4 * ihdr->ihl);
+		if (thdr->dest != htons(tpriv->packet->dport))
+			goto out;
+
+		shdr = (struct netsfhdr *)((u8 *)thdr + sizeof(*thdr));
+	} else {
+		if (ihdr->protocol != IPPROTO_UDP)
+			goto out;
+
+		uhdr = (struct udphdr *)((u8 *)ihdr + 4 * ihdr->ihl);
+		if (uhdr->dest != htons(tpriv->packet->dport))
+			goto out;
+
+		shdr = (struct netsfhdr *)((u8 *)uhdr + sizeof(*uhdr));
+	}
+
+	if (shdr->magic != cpu_to_be64(NET_TEST_PKT_MAGIC))
+		goto out;
+	if (tpriv->packet->id != shdr->id)
+		goto out;
+
+	tpriv->ok = true;
+	complete(&tpriv->comp);
+out:
+	kfree_skb(skb);
+	return 0;
+}
+
+static int __net_test_loopback(struct net_device *ndev,
+			       struct net_packet_attrs *attr)
+{
+	struct net_test_priv *tpriv;
+	struct sk_buff *skb = NULL;
+	int ret = 0;
+
+	tpriv = kzalloc(sizeof(*tpriv), GFP_KERNEL);
+	if (!tpriv)
+		return -ENOMEM;
+
+	tpriv->ok = false;
+	init_completion(&tpriv->comp);
+
+	tpriv->pt.type = htons(ETH_P_IP);
+	tpriv->pt.func = net_test_loopback_validate;
+	tpriv->pt.dev = ndev;
+	tpriv->pt.af_packet_priv = tpriv;
+	tpriv->packet = attr;
+	dev_add_pack(&tpriv->pt);
+
+	skb = net_test_get_skb(ndev, attr);
+	if (!skb) {
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
+	ret = dev_direct_xmit(skb, attr->queue_mapping);
+	if (ret < 0) {
+		goto cleanup;
+	} else if (ret > 0) {
+		ret = -ENETUNREACH;
+		goto cleanup;
+	}
+
+	if (!attr->timeout)
+		attr->timeout = NET_LB_TIMEOUT;
+
+	wait_for_completion_timeout(&tpriv->comp, attr->timeout);
+	ret = tpriv->ok ? 0 : -ETIMEDOUT;
+
+cleanup:
+	dev_remove_pack(&tpriv->pt);
+	kfree(tpriv);
+	return ret;
+}
+
+static int net_test_netif_carrier(struct net_device *ndev)
+{
+	return netif_carrier_ok(ndev) ? 0 : -ENOLINK;
+}
+
+static int net_test_phy_phydev(struct net_device *ndev)
+{
+	return ndev->phydev ? 0 : -EOPNOTSUPP;
+}
+
+static int net_test_phy_loopback_enable(struct net_device *ndev)
+{
+	if (!ndev->phydev)
+		return -EOPNOTSUPP;
+
+	return phy_loopback(ndev->phydev, true);
+}
+
+static int net_test_phy_loopback_disable(struct net_device *ndev)
+{
+	if (!ndev->phydev)
+		return -EOPNOTSUPP;
+
+	return phy_loopback(ndev->phydev, false);
+}
+
+static int net_test_phy_loopback_udp(struct net_device *ndev)
+{
+	struct net_packet_attrs attr = { };
+
+	attr.dst = ndev->dev_addr;
+	return __net_test_loopback(ndev, &attr);
+}
+
+static int net_test_phy_loopback_tcp(struct net_device *ndev)
+{
+	struct net_packet_attrs attr = { };
+
+	attr.dst = ndev->dev_addr;
+	attr.tcp = true;
+	return __net_test_loopback(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, 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,
+	},
+};
+
+void net_selftest(struct net_device *ndev, struct ethtool_test *etest, u64 *buf)
+{
+	int count = net_selftest_get_count();
+	int i;
+
+	memset(buf, 0, sizeof(*buf) * count);
+	net_test_next_id = 0;
+
+	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);
+		if (buf[i] && (buf[i] != -EOPNOTSUPP))
+			etest->flags |= ETH_TEST_FL_FAILED;
+	}
+}
+EXPORT_SYMBOL_GPL(net_selftest);
+
+int net_selftest_get_count(void)
+{
+	return ARRAY_SIZE(net_selftests);
+}
+EXPORT_SYMBOL_GPL(net_selftest_get_count);
+
+void net_selftest_get_strings(u8 *data)
+{
+	u8 *p = data;
+	int i;
+
+	for (i = 0; i < net_selftest_get_count(); i++) {
+		snprintf(p, ETH_GSTRING_LEN, "%2d. %s", i + 1,
+			 net_selftests[i].name);
+		p += ETH_GSTRING_LEN;
+	}
+}
+EXPORT_SYMBOL_GPL(net_selftest_get_strings);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Oleksij Rempel <o.rempel@pengutronix.de>");
-- 
2.29.2

Re: [PATCH net-next v3 3/6] net: add generic selftest support

[Geert Uytterhoeven]

Hi Oleksij,

On Mon, Apr 19, 2021 at 3:13 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> Port some parts of the stmmac selftest and reuse it as basic generic selftest
> library. This patch was tested with following combinations:
> - iMX6DL FEC -> AT8035
> - iMX6DL FEC -> SJA1105Q switch -> KSZ8081
> - iMX6DL FEC -> SJA1105Q switch -> KSZ9031
> - AR9331 ag71xx -> AR9331 PHY
> - AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
generic selftest support") upstream.

> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -429,6 +429,10 @@ config GRO_CELLS
>  config SOCK_VALIDATE_XMIT
>         bool
>
> +config NET_SELFTESTS
> +       def_tristate PHYLIB

Why does this default to enabled if PHYLIB=y?
Usually we allow the user to make selftests modular, independent of the
feature under test, but I may misunderstand the purpose of this test.

Thanks for your clarification!

> +       depends on PHYLIB
> +

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH net-next v3 3/6] net: add generic selftest support

[Oleksij Rempel]

Hi Geert,

On Fri, Apr 30, 2021 at 08:45:05AM +0200, Geert Uytterhoeven wrote:
> Hi Oleksij,
> 
> On Mon, Apr 19, 2021 at 3:13 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > Port some parts of the stmmac selftest and reuse it as basic generic selftest
> > library. This patch was tested with following combinations:
> > - iMX6DL FEC -> AT8035
> > - iMX6DL FEC -> SJA1105Q switch -> KSZ8081
> > - iMX6DL FEC -> SJA1105Q switch -> KSZ9031
> > - AR9331 ag71xx -> AR9331 PHY
> > - AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
> >
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
> generic selftest support") upstream.
>
> > --- a/net/Kconfig
> > +++ b/net/Kconfig
> > @@ -429,6 +429,10 @@ config GRO_CELLS
> >  config SOCK_VALIDATE_XMIT
> >         bool
> >
> > +config NET_SELFTESTS
> > +       def_tristate PHYLIB
> 
> Why does this default to enabled if PHYLIB=y?
> Usually we allow the user to make selftests modular, independent of the
> feature under test, but I may misunderstand the purpose of this test.
> 
> Thanks for your clarification!

There is nothing against making optional. Should I do it?

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 v3 3/6] net: add generic selftest support

[Geert Uytterhoeven]

Hi Oleksij,

On Fri, Apr 30, 2021 at 9:26 AM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> On Fri, Apr 30, 2021 at 08:45:05AM +0200, Geert Uytterhoeven wrote:
> > On Mon, Apr 19, 2021 at 3:13 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> > > Port some parts of the stmmac selftest and reuse it as basic generic selftest
> > > library. This patch was tested with following combinations:
> > > - iMX6DL FEC -> AT8035
> > > - iMX6DL FEC -> SJA1105Q switch -> KSZ8081
> > > - iMX6DL FEC -> SJA1105Q switch -> KSZ9031
> > > - AR9331 ag71xx -> AR9331 PHY
> > > - AR9331 ag71xx -> AR9331 switch -> AR9331 PHY
> > >
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >
> > Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
> > generic selftest support") upstream.
> >
> > > --- a/net/Kconfig
> > > +++ b/net/Kconfig
> > > @@ -429,6 +429,10 @@ config GRO_CELLS
> > >  config SOCK_VALIDATE_XMIT
> > >         bool
> > >
> > > +config NET_SELFTESTS
> > > +       def_tristate PHYLIB
> >
> > Why does this default to enabled if PHYLIB=y?
> > Usually we allow the user to make selftests modular, independent of the
> > feature under test, but I may misunderstand the purpose of this test.
> >
> > Thanks for your clarification!
>
> There is nothing against making optional. Should I do it?

Yes please. Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH net-next v3 3/6] net: add generic selftest support

[Andrew Lunn]

> Thanks for your patch, which is now commit 3e1e58d64c3d0a67 ("net: add
> generic selftest support") upstream.
> 
> > --- a/net/Kconfig
> > +++ b/net/Kconfig
> > @@ -429,6 +429,10 @@ config GRO_CELLS
> >  config SOCK_VALIDATE_XMIT
> >         bool
> >
> > +config NET_SELFTESTS
> > +       def_tristate PHYLIB
> 
> Why does this default to enabled if PHYLIB=y?
> Usually we allow the user to make selftests modular, independent of the
> feature under test, but I may misunderstand the purpose of this test.

Maybe there is a misunderstanding here with 'selftest'. The name page
for ethtool(1) says:

       -t --test
              Executes adapter selftest on the specified network device.  Pos‐
              sible test modes are:

           offline
                  Perform  full set of tests, possibly interrupting normal op‐
                  eration during the tests,

           online Perform limited set of tests, not interrupting normal opera‐
                  tion,

           external_lb
                  Perform  full set of tests, as for offline, and additionally
                  an external-loopback test.

This feature has nothing to do with tools/testing/selftests. It
predates that by decades.

     Andrew

[PATCH net-next v3 4/6] net: fec: make use of generic NET_SELFTESTS library

[Oleksij Rempel]

With this patch FEC on iMX will able to run generic net selftests

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/ethernet/freescale/Kconfig    | 1 +
 drivers/net/ethernet/freescale/fec_main.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 3f9175bdce77..3d937b4650b2 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -26,6 +26,7 @@ config FEC
 		   ARCH_MXC || SOC_IMX28 || COMPILE_TEST)
 	default ARCH_MXC || SOC_IMX28 if ARM
 	select CRC32
+	select NET_SELFTESTS
 	select PHYLIB
 	imply PTP_1588_CLOCK
 	help
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 70aea9c274fe..d51b2eb1de71 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -38,6 +38,7 @@
 #include <linux/in.h>
 #include <linux/ip.h>
 #include <net/ip.h>
+#include <net/selftests.h>
 #include <net/tso.h>
 #include <linux/tcp.h>
 #include <linux/udp.h>
@@ -2481,6 +2482,9 @@ static void fec_enet_get_strings(struct net_device *netdev,
 			memcpy(data + i * ETH_GSTRING_LEN,
 				fec_stats[i].name, ETH_GSTRING_LEN);
 		break;
+	case ETH_SS_TEST:
+		net_selftest_get_strings(data);
+		break;
 	}
 }
 
@@ -2489,6 +2493,8 @@ static int fec_enet_get_sset_count(struct net_device *dev, int sset)
 	switch (sset) {
 	case ETH_SS_STATS:
 		return ARRAY_SIZE(fec_stats);
+	case ETH_SS_TEST:
+		return net_selftest_get_count();
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -2740,6 +2746,7 @@ static const struct ethtool_ops fec_enet_ethtool_ops = {
 	.set_wol		= fec_enet_set_wol,
 	.get_link_ksettings	= phy_ethtool_get_link_ksettings,
 	.set_link_ksettings	= phy_ethtool_set_link_ksettings,
+	.self_test		= net_selftest,
 };
 
 static int fec_enet_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
-- 
2.29.2

[PATCH net-next v3 5/6] net: ag71xx: make use of generic NET_SELFTESTS library

[Oleksij Rempel]

With this patch the ag71xx on Atheros AR9331 will able to run generic net
selftests.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/ethernet/atheros/Kconfig  |  1 +
 drivers/net/ethernet/atheros/ag71xx.c | 20 ++++++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/atheros/Kconfig b/drivers/net/ethernet/atheros/Kconfig
index fb803bf92ded..6842b74b0696 100644
--- a/drivers/net/ethernet/atheros/Kconfig
+++ b/drivers/net/ethernet/atheros/Kconfig
@@ -20,6 +20,7 @@ if NET_VENDOR_ATHEROS
 config AG71XX
 	tristate "Atheros AR7XXX/AR9XXX built-in ethernet mac support"
 	depends on ATH79
+	select NET_SELFTESTS
 	select PHYLINK
 	help
 	  If you wish to compile a kernel for AR7XXX/91XXX and enable
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 7352f98123c7..eb067ce978ae 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -37,6 +37,7 @@
 #include <linux/reset.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <net/selftests.h>
 
 /* For our NAPI weight bigger does *NOT* mean better - it means more
  * D-cache misses and lots more wasted cycles than we'll ever
@@ -497,12 +498,17 @@ static int ag71xx_ethtool_set_pauseparam(struct net_device *ndev,
 static void ag71xx_ethtool_get_strings(struct net_device *netdev, u32 sset,
 				       u8 *data)
 {
-	if (sset == ETH_SS_STATS) {
-		int i;
+	int i;
 
+	switch (sset) {
+	case ETH_SS_STATS:
 		for (i = 0; i < ARRAY_SIZE(ag71xx_statistics); i++)
 			memcpy(data + i * ETH_GSTRING_LEN,
 			       ag71xx_statistics[i].name, ETH_GSTRING_LEN);
+		break;
+	case ETH_SS_TEST:
+		net_selftest_get_strings(data);
+		break;
 	}
 }
 
@@ -519,9 +525,14 @@ static void ag71xx_ethtool_get_stats(struct net_device *ndev,
 
 static int ag71xx_ethtool_get_sset_count(struct net_device *ndev, int sset)
 {
-	if (sset == ETH_SS_STATS)
+	switch (sset) {
+	case ETH_SS_STATS:
 		return ARRAY_SIZE(ag71xx_statistics);
-	return -EOPNOTSUPP;
+	case ETH_SS_TEST:
+		return net_selftest_get_count();
+	default:
+		return -EOPNOTSUPP;
+	}
 }
 
 static const struct ethtool_ops ag71xx_ethtool_ops = {
@@ -536,6 +547,7 @@ static const struct ethtool_ops ag71xx_ethtool_ops = {
 	.get_strings			= ag71xx_ethtool_get_strings,
 	.get_ethtool_stats		= ag71xx_ethtool_get_stats,
 	.get_sset_count			= ag71xx_ethtool_get_sset_count,
+	.self_test			= net_selftest,
 };
 
 static int ag71xx_mdio_wait_busy(struct ag71xx *ag)
-- 
2.29.2

[PATCH net-next v3 6/6] net: dsa: enable selftest support for all switches by default

[Oleksij Rempel]

Most of generic selftest should be able to work with probably all ethernet
controllers. The DSA switches are not exception, so enable it by default at
least for DSA.

This patch was tested with SJA1105 and AR9331.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 include/net/dsa.h |  2 ++
 net/dsa/Kconfig   |  1 +
 net/dsa/slave.c   | 21 +++++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 57b2c49f72f4..b4f89522b545 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -577,6 +577,8 @@ struct dsa_switch_ops {
 					 int port, uint64_t *data);
 	void	(*get_stats64)(struct dsa_switch *ds, int port,
 				   struct rtnl_link_stats64 *s);
+	void	(*self_test)(struct dsa_switch *ds, int port,
+			     struct ethtool_test *etest, u64 *data);
 
 	/*
 	 * ethtool Wake-on-LAN
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 8746b07668ae..cbc2bd643ab2 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -9,6 +9,7 @@ menuconfig NET_DSA
 	select NET_SWITCHDEV
 	select PHYLINK
 	select NET_DEVLINK
+	select NET_SELFTESTS
 	help
 	  Say Y if you want to enable support for the hardware switches supported
 	  by the Distributed Switch Architecture.
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 995e0e16f295..e282b422f733 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -15,6 +15,7 @@
 #include <linux/mdio.h>
 #include <net/rtnetlink.h>
 #include <net/pkt_cls.h>
+#include <net/selftests.h>
 #include <net/tc_act/tc_mirred.h>
 #include <linux/if_bridge.h>
 #include <linux/if_hsr.h>
@@ -748,7 +749,10 @@ static void dsa_slave_get_strings(struct net_device *dev,
 		if (ds->ops->get_strings)
 			ds->ops->get_strings(ds, dp->index, stringset,
 					     data + 4 * len);
+	} else if (stringset ==  ETH_SS_TEST) {
+		net_selftest_get_strings(data);
 	}
+
 }
 
 static void dsa_slave_get_ethtool_stats(struct net_device *dev,
@@ -794,11 +798,27 @@ static int dsa_slave_get_sset_count(struct net_device *dev, int sset)
 			count += ds->ops->get_sset_count(ds, dp->index, sset);
 
 		return count;
+	} else if (sset ==  ETH_SS_TEST) {
+		return net_selftest_get_count();
 	}
 
 	return -EOPNOTSUPP;
 }
 
+static void dsa_slave_net_selftest(struct net_device *ndev,
+				   struct ethtool_test *etest, u64 *buf)
+{
+	struct dsa_port *dp = dsa_slave_to_port(ndev);
+	struct dsa_switch *ds = dp->ds;
+
+	if (ds->ops->self_test) {
+		ds->ops->self_test(ds, dp->index, etest, buf);
+		return;
+	}
+
+	net_selftest(ndev, etest, buf);
+}
+
 static void dsa_slave_get_wol(struct net_device *dev, struct ethtool_wolinfo *w)
 {
 	struct dsa_port *dp = dsa_slave_to_port(dev);
@@ -1630,6 +1650,7 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
 	.get_rxnfc		= dsa_slave_get_rxnfc,
 	.set_rxnfc		= dsa_slave_set_rxnfc,
 	.get_ts_info		= dsa_slave_get_ts_info,
+	.self_test		= dsa_slave_net_selftest,
 };
 
 /* legacy way, bypassing the bridge *****************************************/
-- 
2.29.2

RE: [PATCH net-next v3 0/6] provide generic net selftest support

[Joakim Zhang]

Hi Oleksij,

I look both stmmac selftest code and this patch set. For stmmac, if PHY doesn't support loopback, it will fallthrough to MAC loopback.
You provide this generic net selftest support based on PHY loopback, I have a question, is it possible to extend it also support MAC loopback later?

Best Regards,
Joakim Zhang
> -----Original Message-----
> From: Oleksij Rempel <o.rempel@pengutronix.de>
> Sent: 2021年4月19日 21:01
> To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
> <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang
> Duan <fugang.duan@nxp.com>
> Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio
> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell
> King <linux@armlinux.org.uk>; Philippe Schenker
> <philippe.schenker@toradex.com>
> Subject: [PATCH net-next v3 0/6] provide generic net selftest support
> 
> changes v3:
> - make more granular tests
> - enable loopback for all PHYs by default
> - fix allmodconfig build errors
> - poll for link status update after switching to the loopback mode
> 
> changes v2:
> - make generic selftests available for all networking devices.
> - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> - add loopback support on more PHYs.
> 
> This patch set provides diagnostic capabilities for some iMX, ag71xx or any DSA
> based devices. For proper functionality, PHY loopback support is needed.
> So far there is only initial infrastructure with basic tests.
> 
> Oleksij Rempel (6):
>   net: phy: execute genphy_loopback() per default on all PHYs
>   net: phy: genphy_loopback: add link speed configuration
>   net: add generic selftest support
>   net: fec: make use of generic NET_SELFTESTS library
>   net: ag71xx: make use of generic NET_SELFTESTS library
>   net: dsa: enable selftest support for all switches by default
> 
>  drivers/net/ethernet/atheros/Kconfig      |   1 +
>  drivers/net/ethernet/atheros/ag71xx.c     |  20 +-
>  drivers/net/ethernet/freescale/Kconfig    |   1 +
>  drivers/net/ethernet/freescale/fec_main.c |   7 +
>  drivers/net/phy/phy.c                     |   3 +-
>  drivers/net/phy/phy_device.c              |  35 +-
>  include/linux/phy.h                       |   1 +
>  include/net/dsa.h                         |   2 +
>  include/net/selftests.h                   |  12 +
>  net/Kconfig                               |   4 +
>  net/core/Makefile                         |   1 +
>  net/core/selftests.c                      | 400
> ++++++++++++++++++++++
>  net/dsa/Kconfig                           |   1 +
>  net/dsa/slave.c                           |  21 ++
>  14 files changed, 500 insertions(+), 9 deletions(-)  create mode 100644
> include/net/selftests.h  create mode 100644 net/core/selftests.c
> 
> --
> 2.29.2

Re: [PATCH net-next v3 0/6] provide generic net selftest support

[Oleksij Rempel]

Hi Joakim,

On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> 
> Hi Oleksij,
> 
> I look both stmmac selftest code and this patch set. For stmmac, if PHY doesn't support loopback, it will fallthrough to MAC loopback.
> You provide this generic net selftest support based on PHY loopback, I have a question, is it possible to extend it also support MAC loopback later?

Yes. If you have interest and time to implement it, please do.
It should be some kind of generic callback as phy_loopback() and if PHY
and MAC loopbacks are supported we need to tests both variants.

Best regards,
Oleksij

> > -----Original Message-----
> > From: Oleksij Rempel <o.rempel@pengutronix.de>
> > Sent: 2021年4月19日 21:01
> > To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
> > <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang
> > Duan <fugang.duan@nxp.com>
> > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio
> > Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell
> > King <linux@armlinux.org.uk>; Philippe Schenker
> > <philippe.schenker@toradex.com>
> > Subject: [PATCH net-next v3 0/6] provide generic net selftest support
> > 
> > changes v3:
> > - make more granular tests
> > - enable loopback for all PHYs by default
> > - fix allmodconfig build errors
> > - poll for link status update after switching to the loopback mode
> > 
> > changes v2:
> > - make generic selftests available for all networking devices.
> > - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> > - add loopback support on more PHYs.
> > 
> > This patch set provides diagnostic capabilities for some iMX, ag71xx or any DSA
> > based devices. For proper functionality, PHY loopback support is needed.
> > So far there is only initial infrastructure with basic tests.
> > 
> > Oleksij Rempel (6):
> >   net: phy: execute genphy_loopback() per default on all PHYs
> >   net: phy: genphy_loopback: add link speed configuration
> >   net: add generic selftest support
> >   net: fec: make use of generic NET_SELFTESTS library
> >   net: ag71xx: make use of generic NET_SELFTESTS library
> >   net: dsa: enable selftest support for all switches by default
> > 
> >  drivers/net/ethernet/atheros/Kconfig      |   1 +
> >  drivers/net/ethernet/atheros/ag71xx.c     |  20 +-
> >  drivers/net/ethernet/freescale/Kconfig    |   1 +
> >  drivers/net/ethernet/freescale/fec_main.c |   7 +
> >  drivers/net/phy/phy.c                     |   3 +-
> >  drivers/net/phy/phy_device.c              |  35 +-
> >  include/linux/phy.h                       |   1 +
> >  include/net/dsa.h                         |   2 +
> >  include/net/selftests.h                   |  12 +
> >  net/Kconfig                               |   4 +
> >  net/core/Makefile                         |   1 +
> >  net/core/selftests.c                      | 400
> > ++++++++++++++++++++++
> >  net/dsa/Kconfig                           |   1 +
> >  net/dsa/slave.c                           |  21 ++
> >  14 files changed, 500 insertions(+), 9 deletions(-)  create mode 100644
> > include/net/selftests.h  create mode 100644 net/core/selftests.c
> > 
> > --
> > 2.29.2
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
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 v3 0/6] provide generic net selftest support

[Joakim Zhang]

> -----Original Message-----
> From: Oleksij Rempel <o.rempel@pengutronix.de>
> Sent: 2021年4月23日 12:37
> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
> <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang
> Duan <fugang.duan@nxp.com>; kernel@pengutronix.de;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio
> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell
> King <linux@armlinux.org.uk>; Philippe Schenker
> <philippe.schenker@toradex.com>
> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
> 
> Hi Joakim,
> 
> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> >
> > Hi Oleksij,
> >
> > I look both stmmac selftest code and this patch set. For stmmac, if PHY
> doesn't support loopback, it will fallthrough to MAC loopback.
> > You provide this generic net selftest support based on PHY loopback, I have a
> question, is it possible to extend it also support MAC loopback later?
> 
> Yes. If you have interest and time to implement it, please do.
> It should be some kind of generic callback as phy_loopback() and if PHY and
> MAC loopbacks are supported we need to tests both variants.
Hi Oleksij,

Yes, I can try to implement it when I am free, but I still have some questions:
1. Where we place the generic function? Such as mac_loopback().
2. MAC is different from PHY, need program different registers to enable loopback on different SoCs, that means we need get MAC private data from "struct net_device".
So we need a callback for MAC drivers, where we extend this callback? Could be "struct net_device_ops"? Such as ndo_set_loopback?

Best Regards,
Joakim Zhang
> Best regards,
> Oleksij
> 
> > > -----Original Message-----
> > > From: Oleksij Rempel <o.rempel@pengutronix.de>
> > > Sent: 2021年4月19日 21:01
> > > To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian
> > > Fainelli <f.fainelli@gmail.com>; Heiner Kallweit
> > > <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>
> > > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> > > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > Fabio Estevam <festevam@gmail.com>; David Jander
> > > <david@protonic.nl>; Russell King <linux@armlinux.org.uk>; Philippe
> > > Schenker <philippe.schenker@toradex.com>
> > > Subject: [PATCH net-next v3 0/6] provide generic net selftest
> > > support
> > >
> > > changes v3:
> > > - make more granular tests
> > > - enable loopback for all PHYs by default
> > > - fix allmodconfig build errors
> > > - poll for link status update after switching to the loopback mode
> > >
> > > changes v2:
> > > - make generic selftests available for all networking devices.
> > > - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> > > - add loopback support on more PHYs.
> > >
> > > This patch set provides diagnostic capabilities for some iMX, ag71xx
> > > or any DSA based devices. For proper functionality, PHY loopback support is
> needed.
> > > So far there is only initial infrastructure with basic tests.
> > >
> > > Oleksij Rempel (6):
> > >   net: phy: execute genphy_loopback() per default on all PHYs
> > >   net: phy: genphy_loopback: add link speed configuration
> > >   net: add generic selftest support
> > >   net: fec: make use of generic NET_SELFTESTS library
> > >   net: ag71xx: make use of generic NET_SELFTESTS library
> > >   net: dsa: enable selftest support for all switches by default
> > >
> > >  drivers/net/ethernet/atheros/Kconfig      |   1 +
> > >  drivers/net/ethernet/atheros/ag71xx.c     |  20 +-
> > >  drivers/net/ethernet/freescale/Kconfig    |   1 +
> > >  drivers/net/ethernet/freescale/fec_main.c |   7 +
> > >  drivers/net/phy/phy.c                     |   3 +-
> > >  drivers/net/phy/phy_device.c              |  35 +-
> > >  include/linux/phy.h                       |   1 +
> > >  include/net/dsa.h                         |   2 +
> > >  include/net/selftests.h                   |  12 +
> > >  net/Kconfig                               |   4 +
> > >  net/core/Makefile                         |   1 +
> > >  net/core/selftests.c                      | 400
> > > ++++++++++++++++++++++
> > >  net/dsa/Kconfig                           |   1 +
> > >  net/dsa/slave.c                           |  21 ++
> > >  14 files changed, 500 insertions(+), 9 deletions(-)  create mode
> > > 100644 include/net/selftests.h  create mode 100644
> > > net/core/selftests.c
> > >
> > > --
> > > 2.29.2
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;data=04%7
> C0
> >
> 1%7Cqiangqing.zhang%40nxp.com%7C8796bf53e46b4b1be92b08d9061186f9
> %7C686
> >
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637547494614753358%7CU
> nknown%7
> >
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXV
> >
> CI6Mn0%3D%7C1000&amp;sdata=x%2BUFB%2B1Xp0zbR1mG5HDGvqBUvKhX
> VJn337T%2BB
> > D7cO6g%3D&amp;reserved=0
> 
> --
> Pengutronix e.K.                           |
> |
> Steuerwalder Str. 21                       |
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> ngutronix.de%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C87
> 96bf53e46b4b1be92b08d9061186f9%7C686ea1d3bc2b4c6fa92cd99c5c301635
> %7C0%7C0%7C637547494614753358%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C10
> 00&amp;sdata=K2dsGVxEXv%2FtC7p0l4TFlLlaqzzTa6ktrbSdcCJ10J0%3D&amp;
> reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:
> +49-5121-206917-5555 |

Re: [PATCH net-next v3 0/6] provide generic net selftest support

[Oleksij Rempel]

Hi Joakim,

On Tue, Apr 27, 2021 at 04:48:42AM +0000, Joakim Zhang wrote:
> > Hi Joakim,
> > 
> > On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> > >
> > > Hi Oleksij,
> > >
> > > I look both stmmac selftest code and this patch set. For stmmac, if PHY
> > doesn't support loopback, it will fallthrough to MAC loopback.
> > > You provide this generic net selftest support based on PHY loopback, I have a
> > question, is it possible to extend it also support MAC loopback later?
> > 
> > Yes. If you have interest and time to implement it, please do.
> > It should be some kind of generic callback as phy_loopback() and if PHY and
> > MAC loopbacks are supported we need to tests both variants.
> Hi Oleksij,
> 
> Yes, I can try to implement it when I am free, but I still have some questions:
> 1. Where we place the generic function? Such as mac_loopback().
> 2. MAC is different from PHY, need program different registers to enable loopback on different SoCs, that means we need get MAC private data from "struct net_device".

ACK

> So we need a callback for MAC drivers, where we extend this callback? Could be "struct net_device_ops"? Such as ndo_set_loopback?

yes. Sounds good for me. ndo_set_loopback could be implemented for
ethernet controllers, DSA and even CAN. 

Regards,
Oleksij

> > > > -----Original Message-----
> > > > From: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > Sent: 2021年4月19日 21:01
> > > > To: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > > > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian
> > > > Fainelli <f.fainelli@gmail.com>; Heiner Kallweit
> > > > <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>
> > > > Cc: Oleksij Rempel <o.rempel@pengutronix.de>; kernel@pengutronix.de;
> > > > netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > > Fabio Estevam <festevam@gmail.com>; David Jander
> > > > <david@protonic.nl>; Russell King <linux@armlinux.org.uk>; Philippe
> > > > Schenker <philippe.schenker@toradex.com>
> > > > Subject: [PATCH net-next v3 0/6] provide generic net selftest
> > > > support
> > > >
> > > > changes v3:
> > > > - make more granular tests
> > > > - enable loopback for all PHYs by default
> > > > - fix allmodconfig build errors
> > > > - poll for link status update after switching to the loopback mode
> > > >
> > > > changes v2:
> > > > - make generic selftests available for all networking devices.
> > > > - make use of net_selftest* on FEC, ag71xx and all DSA switches.
> > > > - add loopback support on more PHYs.
> > > >
> > > > This patch set provides diagnostic capabilities for some iMX, ag71xx
> > > > or any DSA based devices. For proper functionality, PHY loopback support is
> > needed.
> > > > So far there is only initial infrastructure with basic tests.
> > > >
> > > > Oleksij Rempel (6):
> > > >   net: phy: execute genphy_loopback() per default on all PHYs
> > > >   net: phy: genphy_loopback: add link speed configuration
> > > >   net: add generic selftest support
> > > >   net: fec: make use of generic NET_SELFTESTS library
> > > >   net: ag71xx: make use of generic NET_SELFTESTS library
> > > >   net: dsa: enable selftest support for all switches by default
> > > >
> > > >  drivers/net/ethernet/atheros/Kconfig      |   1 +
> > > >  drivers/net/ethernet/atheros/ag71xx.c     |  20 +-
> > > >  drivers/net/ethernet/freescale/Kconfig    |   1 +
> > > >  drivers/net/ethernet/freescale/fec_main.c |   7 +
> > > >  drivers/net/phy/phy.c                     |   3 +-
> > > >  drivers/net/phy/phy_device.c              |  35 +-
> > > >  include/linux/phy.h                       |   1 +
> > > >  include/net/dsa.h                         |   2 +
> > > >  include/net/selftests.h                   |  12 +
> > > >  net/Kconfig                               |   4 +
> > > >  net/core/Makefile                         |   1 +
> > > >  net/core/selftests.c                      | 400
> > > > ++++++++++++++++++++++
> > > >  net/dsa/Kconfig                           |   1 +
> > > >  net/dsa/slave.c                           |  21 ++
> > > >  14 files changed, 500 insertions(+), 9 deletions(-)  create mode
> > > > 100644 include/net/selftests.h  create mode 100644
> > > > net/core/selftests.c
> > > >
> > > > --
> > > > 2.29.2
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&amp;data=04%7
> > C0
> > >
> > 1%7Cqiangqing.zhang%40nxp.com%7C8796bf53e46b4b1be92b08d9061186f9
> > %7C686
> > >
> > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637547494614753358%7CU
> > nknown%7
> > >
> > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> > CJXV
> > >
> > CI6Mn0%3D%7C1000&amp;sdata=x%2BUFB%2B1Xp0zbR1mG5HDGvqBUvKhX
> > VJn337T%2BB
> > > D7cO6g%3D&amp;reserved=0
> > 
> > --
> > Pengutronix e.K.                           |
> > |
> > Steuerwalder Str. 21                       |
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe
> > ngutronix.de%2F&amp;data=04%7C01%7Cqiangqing.zhang%40nxp.com%7C87
> > 96bf53e46b4b1be92b08d9061186f9%7C686ea1d3bc2b4c6fa92cd99c5c301635
> > %7C0%7C0%7C637547494614753358%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C10
> > 00&amp;sdata=K2dsGVxEXv%2FtC7p0l4TFlLlaqzzTa6ktrbSdcCJ10J0%3D&amp;
> > reserved=0  |
> > 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0
> > |
> > Amtsgericht Hildesheim, HRA 2686           | Fax:
> > +49-5121-206917-5555 |

-- 
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 v3 0/6] provide generic net selftest support

[Florian Fainelli]

On 4/26/2021 9:48 PM, Joakim Zhang wrote:
> 
>> -----Original Message-----
>> From: Oleksij Rempel <o.rempel@pengutronix.de>
>> Sent: 2021年4月23日 12:37
>> To: Joakim Zhang <qiangqing.zhang@nxp.com>
>> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
>> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli
>> <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; Fugang
>> Duan <fugang.duan@nxp.com>; kernel@pengutronix.de;
>> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio
>> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>; Russell
>> King <linux@armlinux.org.uk>; Philippe Schenker
>> <philippe.schenker@toradex.com>
>> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
>>
>> Hi Joakim,
>>
>> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
>>>
>>> Hi Oleksij,
>>>
>>> I look both stmmac selftest code and this patch set. For stmmac, if PHY
>> doesn't support loopback, it will fallthrough to MAC loopback.
>>> You provide this generic net selftest support based on PHY loopback, I have a
>> question, is it possible to extend it also support MAC loopback later?
>>
>> Yes. If you have interest and time to implement it, please do.
>> It should be some kind of generic callback as phy_loopback() and if PHY and
>> MAC loopbacks are supported we need to tests both variants.
> Hi Oleksij,
> 
> Yes, I can try to implement it when I am free, but I still have some questions:
> 1. Where we place the generic function? Such as mac_loopback().
> 2. MAC is different from PHY, need program different registers to enable loopback on different SoCs, that means we need get MAC private data from "struct net_device".
> So we need a callback for MAC drivers, where we extend this callback? Could be "struct net_device_ops"? Such as ndo_set_loopback?

Even for PHY devices, if we implemented external PHY loopback in the
future, the programming would be different from one vendor to another. I
am starting to wonder if the existing ethtool self-tests are the best
API to expose the ability for an user to perform PHY and MAC loopback
testing.

From an Ethernet MAC and PHY driver perspective, what I would imagine we
could have for a driver API is:

enum ethtool_loopback_mode {
	ETHTOOL_LOOPBACK_OFF,
	ETHTOOL_LOOPBACK_PHY_INTERNAL,
	ETHTOOL_LOOPBACK_PHY_EXTERNAL,
	ETHTOOL_LOOPBACK_MAC_INTERNAL,
	ETHTOOL_LOOPBACK_MAC_EXTERNAL,
	ETHTOOL_LOOPBACK_FIXTURE,
	__ETHTOOL_LOOPBACK_MAX
};

	int (*ndo_set_loopback_mode)(struct net_device *dev, enum
ethtool_loopback_mode mode);

and within the Ethernet MAC driver you would do something like this:

	switch (mode) {
	case ETHTOOL_LOOPBACK_PHY_INTERNAL:
	case ETHTOOL_LOOPBACK_PHY_EXTERNAL:
	case ETHTOOL_LOOPBACK_OFF:
		ret = phy_loopback(ndev->phydev, mode);
		break;
	/* Other case statements implemented in driver */
	
we would need to change the signature of phy_loopback() to accept being
passed ethtool_loopback_mode so we can support different modes.

Whether we want to continue using the self-tests API, or if we implement
a new ethtool command in order to request a loopback operation is up for
discussion.
-- 
Florian

RE: [PATCH net-next v3 0/6] provide generic net selftest support

[Joakim Zhang]

Hi Florian,

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: 2021年4月28日 0:41
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; Oleksij Rempel
> <o.rempel@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Heiner Kallweit
> <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>;
> kernel@pengutronix.de; netdev@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> dl-linux-imx <linux-imx@nxp.com>; Fabio Estevam <festevam@gmail.com>;
> David Jander <david@protonic.nl>; Russell King <linux@armlinux.org.uk>;
> Philippe Schenker <philippe.schenker@toradex.com>
> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
> 
> 
> 
> On 4/26/2021 9:48 PM, Joakim Zhang wrote:
> >
> >> -----Original Message-----
> >> From: Oleksij Rempel <o.rempel@pengutronix.de>
> >> Sent: 2021年4月23日 12:37
> >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> >> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> >> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian
> >> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit
> >> <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>;
> >> kernel@pengutronix.de; netdev@vger.kernel.org;
> >> linux-arm-kernel@lists.infradead.org;
> >> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio
> >> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>;
> >> Russell King <linux@armlinux.org.uk>; Philippe Schenker
> >> <philippe.schenker@toradex.com>
> >> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest
> >> support
> >>
> >> Hi Joakim,
> >>
> >> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> >>>
> >>> Hi Oleksij,
> >>>
> >>> I look both stmmac selftest code and this patch set. For stmmac, if
> >>> PHY
> >> doesn't support loopback, it will fallthrough to MAC loopback.
> >>> You provide this generic net selftest support based on PHY loopback,
> >>> I have a
> >> question, is it possible to extend it also support MAC loopback later?
> >>
> >> Yes. If you have interest and time to implement it, please do.
> >> It should be some kind of generic callback as phy_loopback() and if
> >> PHY and MAC loopbacks are supported we need to tests both variants.
> > Hi Oleksij,
> >
> > Yes, I can try to implement it when I am free, but I still have some questions:
> > 1. Where we place the generic function? Such as mac_loopback().
> > 2. MAC is different from PHY, need program different registers to enable
> loopback on different SoCs, that means we need get MAC private data from
> "struct net_device".
> > So we need a callback for MAC drivers, where we extend this callback? Could
> be "struct net_device_ops"? Such as ndo_set_loopback?
> 
> Even for PHY devices, if we implemented external PHY loopback in the future,
> the programming would be different from one vendor to another. I am starting
> to wonder if the existing ethtool self-tests are the best API to expose the ability
> for an user to perform PHY and MAC loopback testing.
> 
> From an Ethernet MAC and PHY driver perspective, what I would imagine we
> could have for a driver API is:
> 
> enum ethtool_loopback_mode {
> 	ETHTOOL_LOOPBACK_OFF,
> 	ETHTOOL_LOOPBACK_PHY_INTERNAL,
> 	ETHTOOL_LOOPBACK_PHY_EXTERNAL,
> 	ETHTOOL_LOOPBACK_MAC_INTERNAL,
> 	ETHTOOL_LOOPBACK_MAC_EXTERNAL,
> 	ETHTOOL_LOOPBACK_FIXTURE,
> 	__ETHTOOL_LOOPBACK_MAX
> };

What's the difference between internal and external loopback for both PHY and MAC? I am not familiar with these concepts. Thanks.

Best Regards,
Joakim Zhang
> 	int (*ndo_set_loopback_mode)(struct net_device *dev, enum
> ethtool_loopback_mode mode);
> 
> and within the Ethernet MAC driver you would do something like this:
> 
> 	switch (mode) {
> 	case ETHTOOL_LOOPBACK_PHY_INTERNAL:
> 	case ETHTOOL_LOOPBACK_PHY_EXTERNAL:
> 	case ETHTOOL_LOOPBACK_OFF:
> 		ret = phy_loopback(ndev->phydev, mode);
> 		break;
> 	/* Other case statements implemented in driver */
> 
> we would need to change the signature of phy_loopback() to accept being
> passed ethtool_loopback_mode so we can support different modes.
> 
> Whether we want to continue using the self-tests API, or if we implement a
> new ethtool command in order to request a loopback operation is up for
> discussion.
> --
> Florian

Re: [PATCH net-next v3 0/6] provide generic net selftest support

[Oleksij Rempel]

On Wed, Apr 28, 2021 at 08:06:05AM +0000, Joakim Zhang wrote:
> 
> Hi Florian,
> 
> > -----Original Message-----
> > From: Florian Fainelli <f.fainelli@gmail.com>
> > Sent: 2021年4月28日 0:41
> > To: Joakim Zhang <qiangqing.zhang@nxp.com>; Oleksij Rempel
> > <o.rempel@pengutronix.de>
> > Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Heiner Kallweit
> > <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>;
> > kernel@pengutronix.de; netdev@vger.kernel.org;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > dl-linux-imx <linux-imx@nxp.com>; Fabio Estevam <festevam@gmail.com>;
> > David Jander <david@protonic.nl>; Russell King <linux@armlinux.org.uk>;
> > Philippe Schenker <philippe.schenker@toradex.com>
> > Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest support
> > 
> > 
> > 
> > On 4/26/2021 9:48 PM, Joakim Zhang wrote:
> > >
> > >> -----Original Message-----
> > >> From: Oleksij Rempel <o.rempel@pengutronix.de>
> > >> Sent: 2021年4月23日 12:37
> > >> To: Joakim Zhang <qiangqing.zhang@nxp.com>
> > >> Cc: Shawn Guo <shawnguo@kernel.org>; Sascha Hauer
> > >> <s.hauer@pengutronix.de>; Andrew Lunn <andrew@lunn.ch>; Florian
> > >> Fainelli <f.fainelli@gmail.com>; Heiner Kallweit
> > >> <hkallweit1@gmail.com>; Fugang Duan <fugang.duan@nxp.com>;
> > >> kernel@pengutronix.de; netdev@vger.kernel.org;
> > >> linux-arm-kernel@lists.infradead.org;
> > >> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Fabio
> > >> Estevam <festevam@gmail.com>; David Jander <david@protonic.nl>;
> > >> Russell King <linux@armlinux.org.uk>; Philippe Schenker
> > >> <philippe.schenker@toradex.com>
> > >> Subject: Re: [PATCH net-next v3 0/6] provide generic net selftest
> > >> support
> > >>
> > >> Hi Joakim,
> > >>
> > >> On Fri, Apr 23, 2021 at 03:18:32AM +0000, Joakim Zhang wrote:
> > >>>
> > >>> Hi Oleksij,
> > >>>
> > >>> I look both stmmac selftest code and this patch set. For stmmac, if
> > >>> PHY
> > >> doesn't support loopback, it will fallthrough to MAC loopback.
> > >>> You provide this generic net selftest support based on PHY loopback,
> > >>> I have a
> > >> question, is it possible to extend it also support MAC loopback later?
> > >>
> > >> Yes. If you have interest and time to implement it, please do.
> > >> It should be some kind of generic callback as phy_loopback() and if
> > >> PHY and MAC loopbacks are supported we need to tests both variants.
> > > Hi Oleksij,
> > >
> > > Yes, I can try to implement it when I am free, but I still have some questions:
> > > 1. Where we place the generic function? Such as mac_loopback().
> > > 2. MAC is different from PHY, need program different registers to enable
> > loopback on different SoCs, that means we need get MAC private data from
> > "struct net_device".
> > > So we need a callback for MAC drivers, where we extend this callback? Could
> > be "struct net_device_ops"? Such as ndo_set_loopback?
> > 
> > Even for PHY devices, if we implemented external PHY loopback in the future,
> > the programming would be different from one vendor to another. I am starting
> > to wonder if the existing ethtool self-tests are the best API to expose the ability
> > for an user to perform PHY and MAC loopback testing.
> > 
> > From an Ethernet MAC and PHY driver perspective, what I would imagine we
> > could have for a driver API is:
> > 
> > enum ethtool_loopback_mode {
> > 	ETHTOOL_LOOPBACK_OFF,
> > 	ETHTOOL_LOOPBACK_PHY_INTERNAL,
> > 	ETHTOOL_LOOPBACK_PHY_EXTERNAL,
> > 	ETHTOOL_LOOPBACK_MAC_INTERNAL,
> > 	ETHTOOL_LOOPBACK_MAC_EXTERNAL,
> > 	ETHTOOL_LOOPBACK_FIXTURE,
> > 	__ETHTOOL_LOOPBACK_MAX
> > };
> 
> What's the difference between internal and external loopback for both PHY and MAC? I am not familiar with these concepts. Thanks.

For example KSZ9031 PHY. It supports two loopback modes. See page 23:
https://ww1.microchip.com/downloads/en/DeviceDoc/00002096E.pdf

TI DP83TC811R-Q1 PHY supports 4 modes. See page 27:
https://www.ti.com/lit/ds/symlink/dp83tc811r-q1.pdf


> Best Regards,
> Joakim Zhang
> > 	int (*ndo_set_loopback_mode)(struct net_device *dev, enum
> > ethtool_loopback_mode mode);
> > 
> > and within the Ethernet MAC driver you would do something like this:
> > 
> > 	switch (mode) {
> > 	case ETHTOOL_LOOPBACK_PHY_INTERNAL:
> > 	case ETHTOOL_LOOPBACK_PHY_EXTERNAL:
> > 	case ETHTOOL_LOOPBACK_OFF:
> > 		ret = phy_loopback(ndev->phydev, mode);
> > 		break;
> > 	/* Other case statements implemented in driver */
> > 
> > we would need to change the signature of phy_loopback() to accept being
> > passed ethtool_loopback_mode so we can support different modes.
> > 
> > Whether we want to continue using the self-tests API, or if we implement a
> > new ethtool command in order to request a loopback operation is up for
> > discussion.
> > --
> > Florian

-- 
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 |