[ Submitting with net-next closed for proper review and testing. ]
This introduces selftests support in stmmac driver. We add 4 basic sanity
checks and MAC loopback support for all cores within the driver. This way
more tests can easily be added in the future and can be run in virtually
any MAC/GMAC/QoS/XGMAC platform.
Having this we can find regressions and missing features in the driver
while at the same time we can check if the IP is correctly working.
We have been using this for some time now and I do have more tests to
submit in the feature. My experience is that although writing the tests
adds more development time, the gain results are obvious.
I let this feature optional within the driver under a Kconfig option.
For this series the output result will be something like this
(e.g. for dwmac1000):
----
# ethtool -t eth0
The test result is PASS
The test extra info:
1. MAC Loopback 0
2. PHY Loopback -95
3. MMC Counters 0
4. EEE -95
5. Hash Filter MC 0
6. Perfect Filter UC 0
7. Flow Control 0
----
(Error code -95 means EOPNOTSUPP in current HW).
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Jose Abreu (11):
net: stmmac: Add MAC loopback callback to HWIF
net: stmmac: dwmac100: Add MAC loopback support
net: stmmac: dwmac1000: Add MAC loopback support
net: stmmac: dwmac4/5: Add MAC loopback support
net: stmmac: dwxgmac2: Add MAC loopback support
net: stmmac: Switch MMC functions to HWIF callbacks
net: stmmac: dwmac1000: Also pass control frames while in promisc mode
net: stmmac: dwmac4/5: Also pass control frames while in promisc mode
net: stmmac: dwxgmac2: Also pass control frames while in promisc mode
net: stmmac: Introduce selftests support
net: stmmac: dwmac1000: Fix Hash Filter
drivers/net/ethernet/stmicro/stmmac/Kconfig | 9 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 1 +
.../net/ethernet/stmicro/stmmac/dwmac1000_core.c | 16 +-
.../net/ethernet/stmicro/stmmac/dwmac100_core.c | 13 +
drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 2 +
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 17 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +
.../net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 15 +-
drivers/net/ethernet/stmicro/stmmac/hwif.c | 9 +
drivers/net/ethernet/stmicro/stmmac/hwif.h | 21 +
drivers/net/ethernet/stmicro/stmmac/mmc.h | 4 -
drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 13 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 22 +
.../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 8 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +-
.../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 743 +++++++++++++++++++++
18 files changed, 889 insertions(+), 13 deletions(-)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
--
2.7.4
In preparation for the addition of stmmac selftests we implement the MAC
loopback callback in dwmac4/5 cores.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 15 +++++++++++++++
2 files changed, 16 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index eb013d54025a..3dddd7902b0f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -160,6 +160,7 @@ enum power_event {
#define GMAC_CONFIG_PS BIT(15)
#define GMAC_CONFIG_FES BIT(14)
#define GMAC_CONFIG_DM BIT(13)
+#define GMAC_CONFIG_LM BIT(12)
#define GMAC_CONFIG_DCRS BIT(9)
#define GMAC_CONFIG_TE BIT(1)
#define GMAC_CONFIG_RE BIT(0)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 7e5d5db0d516..2f1a2a6f9b33 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -701,6 +701,18 @@ static void dwmac4_debug(void __iomem *ioaddr, struct stmmac_extra_stats *x,
x->mac_gmii_rx_proto_engine++;
}
+static void dwmac4_set_mac_loopback(void __iomem *ioaddr, bool enable)
+{
+ u32 value = readl(ioaddr + GMAC_CONFIG);
+
+ if (enable)
+ value |= GMAC_CONFIG_LM;
+ else
+ value &= ~GMAC_CONFIG_LM;
+
+ writel(value, ioaddr + GMAC_CONFIG);
+}
+
const struct stmmac_ops dwmac4_ops = {
.core_init = dwmac4_core_init,
.set_mac = stmmac_set_mac,
@@ -730,6 +742,7 @@ const struct stmmac_ops dwmac4_ops = {
.pcs_get_adv_lp = dwmac4_get_adv_lp,
.debug = dwmac4_debug,
.set_filter = dwmac4_set_filter,
+ .set_mac_loopback = dwmac4_set_mac_loopback,
};
const struct stmmac_ops dwmac410_ops = {
@@ -761,6 +774,7 @@ const struct stmmac_ops dwmac410_ops = {
.pcs_get_adv_lp = dwmac4_get_adv_lp,
.debug = dwmac4_debug,
.set_filter = dwmac4_set_filter,
+ .set_mac_loopback = dwmac4_set_mac_loopback,
};
const struct stmmac_ops dwmac510_ops = {
@@ -797,6 +811,7 @@ const struct stmmac_ops dwmac510_ops = {
.safety_feat_dump = dwmac5_safety_feat_dump,
.rxp_config = dwmac5_rxp_config,
.flex_pps_config = dwmac5_flex_pps_config,
+ .set_mac_loopback = dwmac4_set_mac_loopback,
};
int dwmac4_setup(struct stmmac_priv *priv)
--
2.7.4
We add support for selftests on stmmac driver with 4 basic sanity checks
for now:
- MAC Loopback
- PHY Loopback
- MMC Counters
- EEE
- Hash Filter Multicast
- Perfect Filter Unicast
- Flow Control
This allows for fast tracking of regressions in the driver and helps in
spotting mis-configuration of HW.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/Kconfig | 9 +
drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 22 +
.../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 6 +
.../net/ethernet/stmicro/stmmac/stmmac_selftests.c | 743 +++++++++++++++++++++
5 files changed, 782 insertions(+)
create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index f194235153f9..fd0bb87a41ee 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -12,6 +12,15 @@ config STMMAC_ETH
if STMMAC_ETH
+config STMMAC_SELFTESTS
+ bool "Support for STMMAC Selftests"
+ depends on STMMAC_ETH
+ default n
+ ---help---
+ This adds support for STMMAC Selftests using ethtool. Enable this
+ feature if you are facing problems with your HW and submit the test
+ results to the netdev Mailing List.
+
config STMMAC_PLATFORM
tristate "STMMAC Platform bus support"
depends on STMMAC_ETH
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index c529c21e9bdd..c59926d96bcc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -8,6 +8,8 @@ stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o ring_mode.o \
stmmac_tc.o dwxgmac2_core.o dwxgmac2_dma.o dwxgmac2_descs.o \
$(stmmac-y)
+stmmac-$(CONFIG_STMMAC_SELFTESTS) += stmmac_selftests.o
+
# Ordering matters. Generic driver must be last.
obj-$(CONFIG_STMMAC_PLATFORM) += stmmac-platform.o
obj-$(CONFIG_DWMAC_ANARION) += dwmac-anarion.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index dd95d959c1ce..a16ada8b8507 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -229,4 +229,26 @@ int stmmac_dvr_probe(struct device *device,
void stmmac_disable_eee_mode(struct stmmac_priv *priv);
bool stmmac_eee_init(struct stmmac_priv *priv);
+#if IS_ENABLED(CONFIG_STMMAC_SELFTESTS)
+void stmmac_selftest_run(struct net_device *dev,
+ struct ethtool_test *etest, u64 *buf);
+void stmmac_selftest_get_strings(struct stmmac_priv *priv, u8 *data);
+int stmmac_selftest_get_count(struct stmmac_priv *priv);
+#else
+static inline void stmmac_selftest_run(struct net_device *dev,
+ struct ethtool_test *etest, u64 *buf)
+{
+ /* Not enabled */
+}
+static inline void stmmac_selftest_get_strings(struct stmmac_priv *priv,
+ u8 *data)
+{
+ /* Not enabled */
+}
+static inline int stmmac_selftest_get_count(struct stmmac_priv *priv)
+{
+ return -EOPNOTSUPP;
+}
+#endif /* CONFIG_STMMAC_SELFTESTS */
+
#endif /* __STMMAC_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index b9f29df7e98a..19ffb4fa90a8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -589,6 +589,8 @@ static int stmmac_get_sset_count(struct net_device *netdev, int sset)
}
return len;
+ case ETH_SS_TEST:
+ return stmmac_selftest_get_count(priv);
default:
return -EOPNOTSUPP;
}
@@ -625,6 +627,9 @@ static void stmmac_get_strings(struct net_device *dev, u32 stringset, u8 *data)
p += ETH_GSTRING_LEN;
}
break;
+ case ETH_SS_TEST:
+ stmmac_selftest_get_strings(priv, p);
+ break;
default:
WARN_ON(1);
break;
@@ -890,6 +895,7 @@ static const struct ethtool_ops stmmac_ethtool_ops = {
.nway_reset = phy_ethtool_nway_reset,
.get_pauseparam = stmmac_get_pauseparam,
.set_pauseparam = stmmac_set_pauseparam,
+ .self_test = stmmac_selftest_run,
.get_ethtool_stats = stmmac_get_ethtool_stats,
.get_strings = stmmac_get_strings,
.get_wol = stmmac_get_wol,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
new file mode 100644
index 000000000000..66e2d842864e
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c
@@ -0,0 +1,743 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019 Synopsys, Inc. and/or its affiliates.
+ * stmmac Selftests Support
+ *
+ * Author: Jose Abreu <[email protected]>
+ */
+
+#include <linux/completion.h>
+#include <linux/ethtool.h>
+#include <linux/ip.h>
+#include <linux/phy.h>
+#include <linux/udp.h>
+#include <net/tcp.h>
+#include <net/udp.h>
+#include "stmmac.h"
+
+struct stmmachdr {
+ __be32 version;
+ __be64 magic;
+ u8 id;
+} __packed;
+
+#define STMMAC_TEST_PKT_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr) + \
+ sizeof(struct stmmachdr))
+#define STMMAC_TEST_PKT_MAGIC 0xdeadcafecafedeadULL
+#define STMMAC_LB_TIMEOUT msecs_to_jiffies(200)
+
+struct stmmac_packet_attrs {
+ int vlan;
+ int vlan_id_in;
+ int vlan_id_out;
+ unsigned char *src;
+ unsigned char *dst;
+ u32 ip_src;
+ u32 ip_dst;
+ int tcp;
+ int sport;
+ int dport;
+ u32 exp_hash;
+ int dont_wait;
+ int timeout;
+ int size;
+ int remove_sa;
+ u8 id;
+};
+
+static u8 stmmac_test_next_id;
+
+static struct sk_buff *stmmac_test_get_udp_skb(struct stmmac_priv *priv,
+ struct stmmac_packet_attrs *attr)
+{
+ struct sk_buff *skb = NULL;
+ struct udphdr *uhdr = NULL;
+ struct tcphdr *thdr = NULL;
+ struct stmmachdr *shdr;
+ struct ethhdr *ehdr;
+ struct iphdr *ihdr;
+ int iplen, size;
+
+ size = attr->size + STMMAC_TEST_PKT_SIZE;
+ if (attr->vlan) {
+ size += 4;
+ if (attr->vlan > 1)
+ size += 4;
+ }
+
+ if (attr->tcp)
+ size += sizeof(struct tcphdr);
+ else
+ size += sizeof(struct udphdr);
+
+ skb = netdev_alloc_skb(priv->dev, size);
+ if (!skb)
+ return NULL;
+
+ prefetchw(skb->data);
+ skb_reserve(skb, NET_IP_ALIGN);
+
+ if (attr->vlan > 1)
+ ehdr = skb_push(skb, ETH_HLEN + 8);
+ else if (attr->vlan)
+ ehdr = skb_push(skb, ETH_HLEN + 4);
+ else if (attr->remove_sa)
+ ehdr = skb_push(skb, ETH_HLEN - 6);
+ else
+ 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));
+
+ if (!attr->remove_sa)
+ eth_zero_addr(ehdr->h_source);
+ eth_zero_addr(ehdr->h_dest);
+ if (attr->src && !attr->remove_sa)
+ ether_addr_copy(ehdr->h_source, attr->src);
+ if (attr->dst)
+ ether_addr_copy(ehdr->h_dest, attr->dst);
+
+ if (!attr->remove_sa) {
+ ehdr->h_proto = htons(ETH_P_IP);
+ } else {
+ __be16 *ptr = (__be16 *)ehdr;
+
+ /* HACK */
+ ptr[3] = htons(ETH_P_IP);
+ }
+
+ if (attr->vlan) {
+ u16 *tag, *proto;
+
+ if (!attr->remove_sa) {
+ tag = (void *)ehdr + ETH_HLEN;
+ proto = (void *)ehdr + (2 * ETH_ALEN);
+ } else {
+ tag = (void *)ehdr + ETH_HLEN - 6;
+ proto = (void *)ehdr + ETH_ALEN;
+ }
+
+ proto[0] = htons(ETH_P_8021Q);
+ tag[0] = htons(attr->vlan_id_out);
+ tag[1] = htons(ETH_P_IP);
+ if (attr->vlan > 1) {
+ proto[0] = htons(ETH_P_8021AD);
+ tag[1] = htons(ETH_P_8021Q);
+ tag[2] = htons(attr->vlan_id_in);
+ tag[3] = 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);
+ 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);
+ ihdr->tot_len = htons(iplen);
+ ihdr->frag_off = 0;
+ ihdr->saddr = 0;
+ 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(STMMAC_TEST_PKT_MAGIC);
+ attr->id = stmmac_test_next_id;
+ shdr->id = stmmac_test_next_id++;
+
+ if (attr->size)
+ skb_put(skb, attr->size);
+
+ 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 = priv->dev;
+
+ return skb;
+}
+
+struct stmmac_test_priv {
+ struct stmmac_packet_attrs *packet;
+ struct packet_type pt;
+ struct completion comp;
+ int double_vlan;
+ int vlan_id;
+ int ok;
+};
+
+static int stmmac_test_loopback_validate(struct sk_buff *skb,
+ struct net_device *ndev,
+ struct packet_type *pt,
+ struct net_device *orig_ndev)
+{
+ struct stmmac_test_priv *tpriv = pt->af_packet_priv;
+ struct stmmachdr *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) < (STMMAC_TEST_PKT_SIZE - ETH_HLEN))
+ goto out;
+
+ ehdr = (struct ethhdr *)skb_mac_header(skb);
+ if (tpriv->packet->dst) {
+ if (!ether_addr_equal(ehdr->h_dest, tpriv->packet->dst))
+ goto out;
+ }
+ if (tpriv->packet->src) {
+ if (!ether_addr_equal(ehdr->h_source, orig_ndev->dev_addr))
+ 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 stmmachdr *)((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 stmmachdr *)((u8 *)uhdr + sizeof(*uhdr));
+ }
+
+ if (shdr->magic != cpu_to_be64(STMMAC_TEST_PKT_MAGIC))
+ goto out;
+ if (tpriv->packet->exp_hash && !skb->hash)
+ goto out;
+ if (tpriv->packet->id != shdr->id)
+ goto out;
+
+ tpriv->ok = true;
+ complete(&tpriv->comp);
+out:
+ kfree_skb(skb);
+ return 0;
+}
+
+static int __stmmac_test_loopback(struct stmmac_priv *priv,
+ struct stmmac_packet_attrs *attr)
+{
+ struct stmmac_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 = stmmac_test_loopback_validate;
+ tpriv->pt.dev = priv->dev;
+ tpriv->pt.af_packet_priv = tpriv;
+ tpriv->packet = attr;
+ dev_add_pack(&tpriv->pt);
+
+ skb = stmmac_test_get_udp_skb(priv, attr);
+ if (!skb) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
+ skb_set_queue_mapping(skb, 0);
+ ret = dev_queue_xmit(skb);
+ if (ret)
+ goto cleanup;
+
+ if (attr->dont_wait)
+ goto cleanup;
+
+ if (!attr->timeout)
+ attr->timeout = STMMAC_LB_TIMEOUT;
+
+ wait_for_completion_timeout(&tpriv->comp, attr->timeout);
+ ret = !tpriv->ok;
+
+cleanup:
+ dev_remove_pack(&tpriv->pt);
+ kfree(tpriv);
+ return ret;
+}
+
+static int stmmac_test_loopback(struct stmmac_priv *priv)
+{
+ struct stmmac_packet_attrs attr = { };
+
+ attr.dst = priv->dev->dev_addr;
+ return __stmmac_test_loopback(priv, &attr);
+}
+
+static int stmmac_test_phy_loopback(struct stmmac_priv *priv)
+{
+ int ret;
+
+ if (!priv->dev->phydev)
+ return -EBUSY;
+
+ ret = phy_loopback(priv->dev->phydev, true);
+ if (ret)
+ return ret;
+
+ return stmmac_test_loopback(priv);
+}
+
+static int stmmac_test_mmc(struct stmmac_priv *priv)
+{
+ struct stmmac_counters initial, final;
+ int ret;
+
+ memset(&initial, 0, sizeof(initial));
+ memset(&final, 0, sizeof(final));
+
+ if (!priv->dma_cap.rmon)
+ return -EOPNOTSUPP;
+
+ /* Save previous results into internal struct */
+ stmmac_mmc_read(priv, priv->mmcaddr, &priv->mmc);
+
+ ret = stmmac_test_loopback(priv);
+ if (ret)
+ return ret;
+
+ /* These will be loopback results so no need to save them */
+ stmmac_mmc_read(priv, priv->mmcaddr, &final);
+
+ if (final.mmc_tx_64_octets_gb <= initial.mmc_tx_64_octets_gb)
+ return -1;
+ if (final.mmc_tx_octetcount_gb <= initial.mmc_tx_octetcount_gb)
+ return -1;
+ if (final.mmc_tx_framecount_gb <= initial.mmc_tx_framecount_gb)
+ return -1;
+ if (final.mmc_tx_octetcount_g <= initial.mmc_tx_octetcount_g)
+ return -1;
+ if (final.mmc_tx_framecount_g <= initial.mmc_tx_framecount_g)
+ return -1;
+ if (final.mmc_rx_64_octets_gb <= initial.mmc_rx_64_octets_gb)
+ return -1;
+ if (final.mmc_rx_octetcount_gb <= initial.mmc_rx_octetcount_gb)
+ return -1;
+ if (final.mmc_rx_framecount_gb <= initial.mmc_rx_framecount_gb)
+ return -1;
+ if (final.mmc_rx_octetcount_g <= initial.mmc_rx_octetcount_g)
+ return -1;
+
+ return 0;
+}
+
+static int stmmac_test_eee(struct stmmac_priv *priv)
+{
+ struct stmmac_extra_stats *initial, *final;
+ int timeout = 100;
+ int ret;
+
+ if (!priv->dma_cap.eee || !priv->eee_active)
+ return -EOPNOTSUPP;
+
+ initial = kzalloc(sizeof(*initial), GFP_KERNEL);
+ if (!initial)
+ return -ENOMEM;
+
+ final = kzalloc(sizeof(*final), GFP_KERNEL);
+ if (!final) {
+ ret = -ENOMEM;
+ goto out_free_initial;
+ }
+
+ memcpy(initial, &priv->xstats, sizeof(*initial));
+
+ ret = stmmac_test_loopback(priv);
+ if (ret)
+ goto out_free_final;
+
+ /* We have no traffic in the line so, sooner or later it will go LPI */
+ while (--timeout) {
+ memcpy(final, &priv->xstats, sizeof(*final));
+
+ if (final->irq_tx_path_in_lpi_mode_n >
+ initial->irq_tx_path_in_lpi_mode_n)
+ break;
+ msleep(100);
+ }
+
+ if (!timeout) {
+ ret = -ETIMEDOUT;
+ goto out_free_final;
+ }
+
+ if (final->irq_tx_path_in_lpi_mode_n <=
+ initial->irq_tx_path_in_lpi_mode_n) {
+ ret = -EINVAL;
+ goto out_free_final;
+ }
+
+ if (final->irq_tx_path_exit_lpi_mode_n <=
+ initial->irq_tx_path_exit_lpi_mode_n) {
+ ret = -EINVAL;
+ goto out_free_final;
+ }
+
+out_free_final:
+ kfree(final);
+out_free_initial:
+ kfree(initial);
+ return ret;
+}
+
+static int stmmac_filter_check(struct stmmac_priv *priv)
+{
+ if (!(priv->dev->flags & IFF_PROMISC))
+ return 0;
+
+ netdev_warn(priv->dev, "Test can't be run in promiscuous mode!\n");
+ return 1;
+}
+
+static int stmmac_test_hfilt(struct stmmac_priv *priv)
+{
+ unsigned char gd_addr[ETH_ALEN] = {0x01, 0x0c, 0xcd, 0x04, 0x00, 0x00};
+ unsigned char bd_addr[ETH_ALEN] = {0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b};
+ struct stmmac_packet_attrs attr = { };
+ int ret;
+
+ if (stmmac_filter_check(priv))
+ return -EOPNOTSUPP;
+
+ ret = dev_mc_add(priv->dev, gd_addr);
+ if (ret)
+ return ret;
+
+ attr.dst = gd_addr;
+
+ /* Shall receive packet */
+ ret = __stmmac_test_loopback(priv, &attr);
+ if (ret)
+ goto cleanup;
+
+ attr.dst = bd_addr;
+
+ /* Shall NOT receive packet */
+ ret = __stmmac_test_loopback(priv, &attr);
+ ret = !ret;
+
+cleanup:
+ dev_mc_del(priv->dev, gd_addr);
+ return ret;
+}
+
+static int stmmac_test_pfilt(struct stmmac_priv *priv)
+{
+ unsigned char gd_addr[ETH_ALEN] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06};
+ unsigned char bd_addr[ETH_ALEN] = {0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b};
+ struct stmmac_packet_attrs attr = { };
+ int ret;
+
+ if (stmmac_filter_check(priv))
+ return -EOPNOTSUPP;
+
+ ret = dev_uc_add(priv->dev, gd_addr);
+ if (ret)
+ return ret;
+
+ attr.dst = gd_addr;
+
+ /* Shall receive packet */
+ ret = __stmmac_test_loopback(priv, &attr);
+ if (ret)
+ goto cleanup;
+
+ attr.dst = bd_addr;
+
+ /* Shall NOT receive packet */
+ ret = __stmmac_test_loopback(priv, &attr);
+ ret = !ret;
+
+cleanup:
+ dev_uc_del(priv->dev, gd_addr);
+ return ret;
+}
+
+static int stmmac_test_flowctrl_validate(struct sk_buff *skb,
+ struct net_device *ndev,
+ struct packet_type *pt,
+ struct net_device *orig_ndev)
+{
+ struct stmmac_test_priv *tpriv = pt->af_packet_priv;
+ struct ethhdr *ehdr;
+
+ ehdr = (struct ethhdr *)skb_mac_header(skb);
+ if (!ether_addr_equal(ehdr->h_source, orig_ndev->dev_addr))
+ goto out;
+ if (ehdr->h_proto != htons(ETH_P_PAUSE))
+ goto out;
+
+ tpriv->ok = true;
+ complete(&tpriv->comp);
+out:
+ kfree(skb);
+ return 0;
+}
+
+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 stmmac_test_priv *tpriv;
+ unsigned int pkt_count;
+ int i, ret = 0;
+
+ if (!phydev || !phydev->pause)
+ return -EOPNOTSUPP;
+
+ tpriv = kzalloc(sizeof(*tpriv), GFP_KERNEL);
+ if (!tpriv)
+ return -ENOMEM;
+
+ tpriv->ok = false;
+ init_completion(&tpriv->comp);
+ tpriv->pt.type = htons(ETH_P_PAUSE);
+ tpriv->pt.func = stmmac_test_flowctrl_validate;
+ tpriv->pt.dev = priv->dev;
+ tpriv->pt.af_packet_priv = tpriv;
+ dev_add_pack(&tpriv->pt);
+
+ /* Compute minimum number of packets to make FIFO full */
+ pkt_count = priv->plat->rx_fifo_size;
+ if (!pkt_count)
+ pkt_count = priv->dma_cap.rx_fifo_size;
+ pkt_count /= 1400;
+ pkt_count *= 2;
+
+ for (i = 0; i < rx_cnt; i++)
+ stmmac_stop_rx(priv, priv->ioaddr, i);
+
+ ret = dev_set_promiscuity(priv->dev, 1);
+ if (ret)
+ goto cleanup;
+
+ ret = dev_mc_add(priv->dev, paddr);
+ if (ret)
+ goto cleanup;
+
+ for (i = 0; i < pkt_count; i++) {
+ struct stmmac_packet_attrs attr = { };
+
+ attr.dst = priv->dev->dev_addr;
+ attr.dont_wait = true;
+ attr.size = 1400;
+
+ ret = __stmmac_test_loopback(priv, &attr);
+ if (ret)
+ goto cleanup;
+ if (tpriv->ok)
+ break;
+ }
+
+ /* Wait for some time in case RX Watchdog is enabled */
+ msleep(200);
+
+ for (i = 0; i < rx_cnt; i++) {
+ struct stmmac_channel *ch = &priv->channel[i];
+
+ stmmac_start_rx(priv, priv->ioaddr, i);
+ napi_schedule(&ch->rx_napi);
+ }
+
+ wait_for_completion_timeout(&tpriv->comp, STMMAC_LB_TIMEOUT);
+ ret = !tpriv->ok;
+
+cleanup:
+ dev_mc_del(priv->dev, paddr);
+ dev_set_promiscuity(priv->dev, -1);
+ dev_remove_pack(&tpriv->pt);
+ kfree(tpriv);
+ 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;
+ int (*fn)(struct stmmac_priv *priv);
+} stmmac_selftests[] = {
+ {
+ .name = "MAC Loopback ",
+ .lb = STMMAC_LOOPBACK_MAC,
+ .fn = stmmac_test_loopback,
+ }, {
+ .name = "PHY Loopback ",
+ .lb = STMMAC_LOOPBACK_PHY,
+ .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 = "Flow Control ",
+ .lb = STMMAC_LOOPBACK_PHY,
+ .fn = stmmac_test_flowctrl,
+ },
+};
+
+void stmmac_selftest_run(struct net_device *dev,
+ struct ethtool_test *etest, u64 *buf)
+{
+ struct stmmac_priv *priv = netdev_priv(dev);
+ int count = stmmac_selftest_get_count(priv);
+ int carrier = netif_carrier_ok(dev);
+ int i, ret;
+
+ memset(buf, 0, sizeof(*buf) * count);
+ stmmac_test_next_id = 0;
+
+ /* We don't want extra traffic */
+ netif_carrier_off(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);
+ if (!ret)
+ break;
+ /* Fallthrough */
+ case STMMAC_LOOPBACK_MAC:
+ ret = stmmac_set_mac_loopback(priv, priv->ioaddr, true);
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ /*
+ * First tests will always be MAC / PHY loobpack. 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_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);
+ if (!ret)
+ break;
+ /* Fallthrough */
+ case STMMAC_LOOPBACK_MAC:
+ stmmac_set_mac_loopback(priv, priv->ioaddr, false);
+ break;
+ default:
+ break;
+ }
+ }
+
+ /* Restart everything */
+ if (carrier)
+ netif_carrier_on(dev);
+}
+
+void stmmac_selftest_get_strings(struct stmmac_priv *priv, u8 *data)
+{
+ u8 *p = data;
+ int i;
+
+ for (i = 0; i < stmmac_selftest_get_count(priv); i++) {
+ snprintf(p, ETH_GSTRING_LEN, "%2d. %s", i + 1,
+ stmmac_selftests[i].name);
+ p += ETH_GSTRING_LEN;
+ }
+}
+
+int stmmac_selftest_get_count(struct stmmac_priv *priv)
+{
+ return ARRAY_SIZE(stmmac_selftests);
+}
--
2.7.4
In order for the selftests to run the Flow Control selftest we need to
also pass control frames to the stack.
Pass this type of frames while in promiscuous mode.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac1000.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
index 184ca13c8f79..56a69fb6f0b9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000.h
@@ -146,6 +146,7 @@ enum inter_frame_gap {
#define GMAC_FRAME_FILTER_DAIF 0x00000008 /* DA Inverse Filtering */
#define GMAC_FRAME_FILTER_PM 0x00000010 /* Pass all multicast */
#define GMAC_FRAME_FILTER_DBF 0x00000020 /* Disable Broadcast frames */
+#define GMAC_FRAME_FILTER_PCF 0x00000080 /* Pass Control frames */
#define GMAC_FRAME_FILTER_SAIF 0x00000100 /* Inverse Filtering */
#define GMAC_FRAME_FILTER_SAF 0x00000200 /* Source Address Filter */
#define GMAC_FRAME_FILTER_HPF 0x00000400 /* Hash or perfect Filter */
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 398303c783f4..8ca73bd15e07 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -172,7 +172,7 @@ static void dwmac1000_set_filter(struct mac_device_info *hw,
memset(mc_filter, 0, sizeof(mc_filter));
if (dev->flags & IFF_PROMISC) {
- value = GMAC_FRAME_FILTER_PR;
+ value = GMAC_FRAME_FILTER_PR | GMAC_FRAME_FILTER_PCF;
} else if (dev->flags & IFF_ALLMULTI) {
value = GMAC_FRAME_FILTER_PM; /* pass all multi */
} else if (!netdev_mc_empty(dev)) {
--
2.7.4
In order for the selftests to run the Flow Control selftest we need to
also pass control frames to the stack.
Pass this type of frames while in promiscuous mode.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index f629ccc8932a..b8296eb41011 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -40,6 +40,7 @@
#define XGMAC_CORE_INIT_RX 0
#define XGMAC_PACKET_FILTER 0x00000008
#define XGMAC_FILTER_RA BIT(31)
+#define XGMAC_FILTER_PCF BIT(7)
#define XGMAC_FILTER_PM BIT(4)
#define XGMAC_FILTER_HMC BIT(2)
#define XGMAC_FILTER_PR BIT(0)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index c27b3ca052ea..bfa7d6913fd4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -310,7 +310,7 @@ static void dwxgmac2_set_filter(struct mac_device_info *hw,
u32 value = XGMAC_FILTER_RA;
if (dev->flags & IFF_PROMISC) {
- value |= XGMAC_FILTER_PR;
+ value |= XGMAC_FILTER_PR | XGMAC_FILTER_PCF;
} else if ((dev->flags & IFF_ALLMULTI) ||
(netdev_mc_count(dev) > HASH_TABLE_SIZE)) {
value |= XGMAC_FILTER_PM;
--
2.7.4
XGMAC has a different MMC module. Lets use HWIF callbacks for MMC module
so that correct callbacks are automatically selected.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
drivers/net/ethernet/stmicro/stmmac/hwif.c | 9 +++++++++
drivers/net/ethernet/stmicro/stmmac/hwif.h | 17 +++++++++++++++++
drivers/net/ethernet/stmicro/stmmac/mmc.h | 4 ----
drivers/net/ethernet/stmicro/stmmac/mmc_core.c | 13 ++++++++++---
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++--
7 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 272b9ca66314..1961fe9144ca 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -424,6 +424,7 @@ struct mac_device_info {
const struct stmmac_mode_ops *mode;
const struct stmmac_hwtimestamp *ptp;
const struct stmmac_tc_ops *tc;
+ const struct stmmac_mmc_ops *mmc;
struct mii_regs mii; /* MII register Addresses */
struct mac_link link;
void __iomem *pcsr; /* vpointer to device CSRs */
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
index 81b966a8261b..6c61b753b55e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
@@ -81,6 +81,7 @@ static const struct stmmac_hwif_entry {
const void *hwtimestamp;
const void *mode;
const void *tc;
+ const void *mmc;
int (*setup)(struct stmmac_priv *priv);
int (*quirks)(struct stmmac_priv *priv);
} stmmac_hw[] = {
@@ -100,6 +101,7 @@ static const struct stmmac_hwif_entry {
.hwtimestamp = &stmmac_ptp,
.mode = NULL,
.tc = NULL,
+ .mmc = &dwmac_mmc_ops,
.setup = dwmac100_setup,
.quirks = stmmac_dwmac1_quirks,
}, {
@@ -117,6 +119,7 @@ static const struct stmmac_hwif_entry {
.hwtimestamp = &stmmac_ptp,
.mode = NULL,
.tc = NULL,
+ .mmc = &dwmac_mmc_ops,
.setup = dwmac1000_setup,
.quirks = stmmac_dwmac1_quirks,
}, {
@@ -134,6 +137,7 @@ static const struct stmmac_hwif_entry {
.hwtimestamp = &stmmac_ptp,
.mode = NULL,
.tc = &dwmac510_tc_ops,
+ .mmc = &dwmac_mmc_ops,
.setup = dwmac4_setup,
.quirks = stmmac_dwmac4_quirks,
}, {
@@ -151,6 +155,7 @@ static const struct stmmac_hwif_entry {
.hwtimestamp = &stmmac_ptp,
.mode = &dwmac4_ring_mode_ops,
.tc = &dwmac510_tc_ops,
+ .mmc = &dwmac_mmc_ops,
.setup = dwmac4_setup,
.quirks = NULL,
}, {
@@ -168,6 +173,7 @@ static const struct stmmac_hwif_entry {
.hwtimestamp = &stmmac_ptp,
.mode = &dwmac4_ring_mode_ops,
.tc = &dwmac510_tc_ops,
+ .mmc = &dwmac_mmc_ops,
.setup = dwmac4_setup,
.quirks = NULL,
}, {
@@ -185,6 +191,7 @@ static const struct stmmac_hwif_entry {
.hwtimestamp = &stmmac_ptp,
.mode = &dwmac4_ring_mode_ops,
.tc = &dwmac510_tc_ops,
+ .mmc = &dwmac_mmc_ops,
.setup = dwmac4_setup,
.quirks = NULL,
}, {
@@ -202,6 +209,7 @@ static const struct stmmac_hwif_entry {
.hwtimestamp = &stmmac_ptp,
.mode = NULL,
.tc = &dwmac510_tc_ops,
+ .mmc = NULL,
.setup = dwxgmac2_setup,
.quirks = NULL,
},
@@ -267,6 +275,7 @@ int stmmac_hwif_init(struct stmmac_priv *priv)
mac->ptp = mac->ptp ? : entry->hwtimestamp;
mac->mode = mac->mode ? : entry->mode;
mac->tc = mac->tc ? : entry->tc;
+ mac->mmc = mac->mmc ? : entry->mmc;
priv->hw = mac;
priv->ptpaddr = priv->ioaddr + entry->regs.ptp_off;
diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.h b/drivers/net/ethernet/stmicro/stmmac/hwif.h
index 9a000dc31d9e..2acfbc70e3c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/hwif.h
+++ b/drivers/net/ethernet/stmicro/stmmac/hwif.h
@@ -6,6 +6,7 @@
#define __STMMAC_HWIF_H__
#include <linux/netdevice.h>
+#include <linux/stmmac.h>
#define stmmac_do_void_callback(__priv, __module, __cname, __arg0, __args...) \
({ \
@@ -468,6 +469,21 @@ struct stmmac_tc_ops {
#define stmmac_tc_setup_cbs(__priv, __args...) \
stmmac_do_callback(__priv, tc, setup_cbs, __args)
+struct stmmac_counters;
+
+struct stmmac_mmc_ops {
+ void (*ctrl)(void __iomem *ioaddr, unsigned int mode);
+ void (*intr_all_mask)(void __iomem *ioaddr);
+ void (*read)(void __iomem *ioaddr, struct stmmac_counters *mmc);
+};
+
+#define stmmac_mmc_ctrl(__priv, __args...) \
+ stmmac_do_void_callback(__priv, mmc, ctrl, __args)
+#define stmmac_mmc_intr_all_mask(__priv, __args...) \
+ stmmac_do_void_callback(__priv, mmc, intr_all_mask, __args)
+#define stmmac_mmc_read(__priv, __args...) \
+ stmmac_do_void_callback(__priv, mmc, read, __args)
+
struct stmmac_regs_off {
u32 ptp_off;
u32 mmc_off;
@@ -486,6 +502,7 @@ extern const struct stmmac_tc_ops dwmac510_tc_ops;
extern const struct stmmac_ops dwxgmac210_ops;
extern const struct stmmac_dma_ops dwxgmac210_dma_ops;
extern const struct stmmac_desc_ops dwxgmac210_desc_ops;
+extern const struct stmmac_mmc_ops dwmac_mmc_ops;
#define GMAC_VERSION 0x00000020 /* GMAC CORE Version */
#define GMAC4_VERSION 0x00000110 /* GMAC4+ CORE Version */
diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc.h b/drivers/net/ethernet/stmicro/stmmac/mmc.h
index c037326331f5..e2bd90a4d34f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/mmc.h
+++ b/drivers/net/ethernet/stmicro/stmmac/mmc.h
@@ -128,8 +128,4 @@ struct stmmac_counters {
unsigned int mmc_rx_icmp_err_octets;
};
-void dwmac_mmc_ctrl(void __iomem *ioaddr, unsigned int mode);
-void dwmac_mmc_intr_all_mask(void __iomem *ioaddr);
-void dwmac_mmc_read(void __iomem *ioaddr, struct stmmac_counters *mmc);
-
#endif /* __MMC_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
index e9b04c28980f..b8c598125cfe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/mmc_core.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/io.h>
+#include "hwif.h"
#include "mmc.h"
/* MAC Management Counters register offset */
@@ -128,7 +129,7 @@
#define MMC_RX_ICMP_GD_OCTETS 0x180
#define MMC_RX_ICMP_ERR_OCTETS 0x184
-void dwmac_mmc_ctrl(void __iomem *mmcaddr, unsigned int mode)
+static void dwmac_mmc_ctrl(void __iomem *mmcaddr, unsigned int mode)
{
u32 value = readl(mmcaddr + MMC_CNTRL);
@@ -141,7 +142,7 @@ void dwmac_mmc_ctrl(void __iomem *mmcaddr, unsigned int mode)
}
/* To mask all all interrupts.*/
-void dwmac_mmc_intr_all_mask(void __iomem *mmcaddr)
+static void dwmac_mmc_intr_all_mask(void __iomem *mmcaddr)
{
writel(MMC_DEFAULT_MASK, mmcaddr + MMC_RX_INTR_MASK);
writel(MMC_DEFAULT_MASK, mmcaddr + MMC_TX_INTR_MASK);
@@ -153,7 +154,7 @@ void dwmac_mmc_intr_all_mask(void __iomem *mmcaddr)
* counter after a read. So all the field of the mmc struct
* have to be incremented.
*/
-void dwmac_mmc_read(void __iomem *mmcaddr, struct stmmac_counters *mmc)
+static void dwmac_mmc_read(void __iomem *mmcaddr, struct stmmac_counters *mmc)
{
mmc->mmc_tx_octetcount_gb += readl(mmcaddr + MMC_TX_OCTETCOUNT_GB);
mmc->mmc_tx_framecount_gb += readl(mmcaddr + MMC_TX_FRAMECOUNT_GB);
@@ -266,3 +267,9 @@ void dwmac_mmc_read(void __iomem *mmcaddr, struct stmmac_counters *mmc)
mmc->mmc_rx_icmp_gd_octets += readl(mmcaddr + MMC_RX_ICMP_GD_OCTETS);
mmc->mmc_rx_icmp_err_octets += readl(mmcaddr + MMC_RX_ICMP_ERR_OCTETS);
}
+
+const struct stmmac_mmc_ops dwmac_mmc_ops = {
+ .ctrl = dwmac_mmc_ctrl,
+ .intr_all_mask = dwmac_mmc_intr_all_mask,
+ .read = dwmac_mmc_read,
+};
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 3c749c327cbd..b9f29df7e98a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -537,7 +537,7 @@ static void stmmac_get_ethtool_stats(struct net_device *dev,
if (ret) {
/* If supported, for new GMAC chips expose the MMC counters */
if (priv->dma_cap.rmon) {
- dwmac_mmc_read(priv->mmcaddr, &priv->mmc);
+ stmmac_mmc_read(priv, priv->mmcaddr, &priv->mmc);
for (i = 0; i < STMMAC_MMC_STATS_LEN; i++) {
char *p;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 5ab2733e15e2..571b4a619ed6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2128,10 +2128,10 @@ static void stmmac_mmc_setup(struct stmmac_priv *priv)
unsigned int mode = MMC_CNTRL_RESET_ON_READ | MMC_CNTRL_COUNTER_RESET |
MMC_CNTRL_PRESET | MMC_CNTRL_FULL_HALF_PRESET;
- dwmac_mmc_intr_all_mask(priv->mmcaddr);
+ stmmac_mmc_intr_all_mask(priv, priv->mmcaddr);
if (priv->dma_cap.rmon) {
- dwmac_mmc_ctrl(priv->mmcaddr, mode);
+ stmmac_mmc_ctrl(priv, priv->mmcaddr, mode);
memset(&priv->mmc, 0, sizeof(struct stmmac_counters));
} else
netdev_info(priv->dev, "No MAC Management Counters available\n");
--
2.7.4
In order for hash filter to work we need to set the HPF bit.
Found out while running stmmac selftests.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
index 8ca73bd15e07..bc91af6c01b6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
@@ -198,6 +198,7 @@ static void dwmac1000_set_filter(struct mac_device_info *hw,
}
}
+ value |= GMAC_FRAME_FILTER_HPF;
dwmac1000_set_mchash(ioaddr, mc_filter, mcbitslog2);
/* Handle multiple unicast addresses (perfect filtering) */
--
2.7.4
In order for the selftests to run the Flow Control selftest we need to
also pass control frames to the stack.
Pass this type of frames while in promiscuous mode.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 3dddd7902b0f..c3cbca804bcd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -64,6 +64,7 @@
#define GMAC_PACKET_FILTER_PR BIT(0)
#define GMAC_PACKET_FILTER_HMC BIT(2)
#define GMAC_PACKET_FILTER_PM BIT(4)
+#define GMAC_PACKET_FILTER_PCF BIT(7)
#define GMAC_MAX_PERFECT_ADDRESSES 128
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index 2f1a2a6f9b33..02a3a7e2db6e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -404,7 +404,7 @@ static void dwmac4_set_filter(struct mac_device_info *hw,
unsigned int value = 0;
if (dev->flags & IFF_PROMISC) {
- value = GMAC_PACKET_FILTER_PR;
+ value = GMAC_PACKET_FILTER_PR | GMAC_PACKET_FILTER_PCF;
} else if ((dev->flags & IFF_ALLMULTI) ||
(netdev_mc_count(dev) > HASH_TABLE_SIZE)) {
/* Pass all multi */
--
2.7.4
In preparation for the addition of stmmac selftests we implement the MAC
loopback callback in dwxgmac2 core.
Signed-off-by: Jose Abreu <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 1 +
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index 085b700a4994..f629ccc8932a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -29,6 +29,7 @@
#define XGMAC_CONFIG_GPSL GENMASK(29, 16)
#define XGMAC_CONFIG_GPSL_SHIFT 16
#define XGMAC_CONFIG_S2KP BIT(11)
+#define XGMAC_CONFIG_LM BIT(10)
#define XGMAC_CONFIG_IPC BIT(9)
#define XGMAC_CONFIG_JE BIT(8)
#define XGMAC_CONFIG_WD BIT(7)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
index 64b8cb88ea45..c27b3ca052ea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_core.c
@@ -321,6 +321,18 @@ static void dwxgmac2_set_filter(struct mac_device_info *hw,
writel(value, ioaddr + XGMAC_PACKET_FILTER);
}
+static void dwxgmac2_set_mac_loopback(void __iomem *ioaddr, bool enable)
+{
+ u32 value = readl(ioaddr + XGMAC_RX_CONFIG);
+
+ if (enable)
+ value |= XGMAC_CONFIG_LM;
+ else
+ value &= ~XGMAC_CONFIG_LM;
+
+ writel(value, ioaddr + XGMAC_RX_CONFIG);
+}
+
const struct stmmac_ops dwxgmac210_ops = {
.core_init = dwxgmac2_core_init,
.set_mac = dwxgmac2_set_mac,
@@ -350,6 +362,7 @@ const struct stmmac_ops dwxgmac210_ops = {
.pcs_get_adv_lp = NULL,
.debug = NULL,
.set_filter = dwxgmac2_set_filter,
+ .set_mac_loopback = dwxgmac2_set_mac_loopback,
};
int dwxgmac2_setup(struct stmmac_priv *priv)
--
2.7.4
From: Andrew Lunn <[email protected]>
Date: Wed, May 08, 2019 at 13:04:58
> Do you mean pause frames?
Yes in order for the test to pass the MAC has to pass the pause frames to
the stack.
Thanks,
Jose Miguel Abreu
On Wed, May 08, 2019 at 09:51:07AM +0200, Jose Abreu wrote:
> In order for the selftests to run the Flow Control selftest we need to
> also pass control frames to the stack.
Hi Jose
Do you mean pause frames?
Thanks
Andrew
From: Jose Abreu <[email protected]>
Date: Wed, 8 May 2019 09:51:00 +0200
> [ Submitting with net-next closed for proper review and testing. ]
Please use "RFC" in the subject line when doing this in the future.
Thank you.
On Wed, May 08, 2019 at 09:51:00AM +0200, Jose Abreu wrote:
> [ Submitting with net-next closed for proper review and testing. ]
>
> This introduces selftests support in stmmac driver. We add 4 basic sanity
> checks and MAC loopback support for all cores within the driver. This way
> more tests can easily be added in the future and can be run in virtually
> any MAC/GMAC/QoS/XGMAC platform.
>
> Having this we can find regressions and missing features in the driver
> while at the same time we can check if the IP is correctly working.
>
> We have been using this for some time now and I do have more tests to
> submit in the feature. My experience is that although writing the tests
> adds more development time, the gain results are obvious.
>
> I let this feature optional within the driver under a Kconfig option.
>
> For this series the output result will be something like this
> (e.g. for dwmac1000):
> ----
> # ethtool -t eth0
> The test result is PASS
> The test extra info:
> 1. MAC Loopback 0
> 2. PHY Loopback -95
> 3. MMC Counters 0
> 4. EEE -95
> 5. Hash Filter MC 0
> 6. Perfect Filter UC 0
> 7. Flow Control 0
Hi Jose
The man page says:
-t --test
Executes adapter selftest on the specified network
device. Possible test modes are:
offline
Perform full set of tests, possibly interrupting
normal operation during the tests,
online Perform limited set of tests, not interrupting
normal operation,
external_lb
Perform full set of tests, as for offline, and
additionally an external-loopback test.
The normal operation is interrupted by the tests you carry out
here. But i don't see any code looking for ETH_TEST_FL_OFFLINE
> (Error code -95 means EOPNOTSUPP in current HW).
How deep do you have to go before you know about EOPNOTSUPP? It would
be better to not return the string and result at all. Or patch ethtool
to call strerror(3).
Andrew
> +static int stmmac_test_eee(struct stmmac_priv *priv)
> +{
> + struct stmmac_extra_stats *initial, *final;
> + int timeout = 100;
> + int ret;
> +
> + ret = stmmac_test_loopback(priv);
> + if (ret)
> + goto out_free_final;
> +
> + /* We have no traffic in the line so, sooner or later it will go LPI */
> + while (--timeout) {
> + memcpy(final, &priv->xstats, sizeof(*final));
> +
> + if (final->irq_tx_path_in_lpi_mode_n >
> + initial->irq_tx_path_in_lpi_mode_n)
> + break;
> + msleep(100);
> + }
> +
> + if (!timeout) {
> + ret = -ETIMEDOUT;
> + goto out_free_final;
> + }
Retries would be a better name than timeout.
Also, 100 * 100 ms seems like a long time.
> +static int stmmac_filter_check(struct stmmac_priv *priv)
> +{
> + if (!(priv->dev->flags & IFF_PROMISC))
> + return 0;
> +
> + netdev_warn(priv->dev, "Test can't be run in promiscuous mode!\n");
> + return 1;
Maybe return EOPNOTSUPP here,
> +}
> +
> +static int stmmac_test_hfilt(struct stmmac_priv *priv)
> +{
> + unsigned char gd_addr[ETH_ALEN] = {0x01, 0x0c, 0xcd, 0x04, 0x00, 0x00};
> + unsigned char bd_addr[ETH_ALEN] = {0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b};
What does gd and bd mean?
> + struct stmmac_packet_attrs attr = { };
> + int ret;
> +
> + if (stmmac_filter_check(priv))
> + return -EOPNOTSUPP;
and just return the error code from the call.
> +
> + ret = dev_mc_add(priv->dev, gd_addr);
> + if (ret)
> + return ret;
> +
> + attr.dst = gd_addr;
> +
> + /* Shall receive packet */
> + ret = __stmmac_test_loopback(priv, &attr);
> + if (ret)
> + goto cleanup;
> +
> + attr.dst = bd_addr;
> +
> + /* Shall NOT receive packet */
> + ret = __stmmac_test_loopback(priv, &attr);
> + ret = !ret;
What is this test testing? gd is a multicast, where as bd is not. I
expect the hardware treats multicast different to unicast. So it would
make more sense to test two different multicast addresses, one which
has been added via dev_mc_addr, and one that has not?
> +
> +cleanup:
> + dev_mc_del(priv->dev, gd_addr);
> + return ret;
> +}
> +
> +static int stmmac_test_pfilt(struct stmmac_priv *priv)
> +{
> + unsigned char gd_addr[ETH_ALEN] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06};
> + unsigned char bd_addr[ETH_ALEN] = {0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b};
> + struct stmmac_packet_attrs attr = { };
> + int ret;
> +
> + if (stmmac_filter_check(priv))
> + return -EOPNOTSUPP;
> +
> + ret = dev_uc_add(priv->dev, gd_addr);
> + if (ret)
> + return ret;
> +
> + attr.dst = gd_addr;
> +
> + /* Shall receive packet */
> + ret = __stmmac_test_loopback(priv, &attr);
> + if (ret)
> + goto cleanup;
gb is a multicast address. Does dev_uc_add() return an error? If it
does not we should not expect it to actually work, since a multicast
address should not match a unicast address?
You also seem to be missing a test for adding a unicast address via
dev_uc_add() and receiving packets for that address, but not receiving
multicast packets.
> +static const struct stmmac_test {
> + char name[ETH_GSTRING_LEN];
> + int lb;
> + int (*fn)(struct stmmac_priv *priv);
> +} stmmac_selftests[] = {
> + {
> + .name = "MAC Loopback ",
> + .lb = STMMAC_LOOPBACK_MAC,
> + .fn = stmmac_test_loopback,
stmmac_test_mac_loopback might be a better name.
> + }, {
> + .name = "PHY Loopback ",
> + .lb = STMMAC_LOOPBACK_PHY,
> + .fn = stmmac_test_phy_loopback,
> + }, {
Andrew
From: Andrew Lunn <[email protected]>
Date: Wed, May 08, 2019 at 20:50:11
> The normal operation is interrupted by the tests you carry out
> here. But i don't see any code looking for ETH_TEST_FL_OFFLINE
Ok will fix to only run in offline mode then.
>
> > (Error code -95 means EOPNOTSUPP in current HW).
>
> How deep do you have to go before you know about EOPNOTSUPP? It would
> be better to not return the string and result at all. Or patch ethtool
> to call strerror(3).
When I looked at other drivers I saw that they return positive value (1)
or zero so calling strerror in ethtool may not be ideal.
I think its useful to let the user know if a given test is not supported
in HW so maybe I can return 1 instead of EOPNOTSUPP ?
Thanks,
Jose Miguel Abreu
From: Andrew Lunn <[email protected]>
Date: Thu, May 09, 2019 at 03:23:30
> > +static int stmmac_test_eee(struct stmmac_priv *priv)
> > +{
> > + struct stmmac_extra_stats *initial, *final;
> > + int timeout = 100;
> > + int ret;
> > +
> > + ret = stmmac_test_loopback(priv);
> > + if (ret)
> > + goto out_free_final;
> > +
> > + /* We have no traffic in the line so, sooner or later it will go LPI */
> > + while (--timeout) {
> > + memcpy(final, &priv->xstats, sizeof(*final));
> > +
> > + if (final->irq_tx_path_in_lpi_mode_n >
> > + initial->irq_tx_path_in_lpi_mode_n)
> > + break;
> > + msleep(100);
> > + }
> > +
> > + if (!timeout) {
> > + ret = -ETIMEDOUT;
> > + goto out_free_final;
> > + }
>
> Retries would be a better name than timeout.
Ok.
>
> Also, 100 * 100 ms seems like a long time.
Ah, yeah. I will adjust to 0.5 or maybe 1 sec max.
>
> > +static int stmmac_filter_check(struct stmmac_priv *priv)
> > +{
> > + if (!(priv->dev->flags & IFF_PROMISC))
> > + return 0;
> > +
> > + netdev_warn(priv->dev, "Test can't be run in promiscuous mode!\n");
> > + return 1;
>
> Maybe return EOPNOTSUPP here,
Ok.
>
> > +}
> > +
> > +static int stmmac_test_hfilt(struct stmmac_priv *priv)
> > +{
> > + unsigned char gd_addr[ETH_ALEN] = {0x01, 0x0c, 0xcd, 0x04, 0x00, 0x00};
> > + unsigned char bd_addr[ETH_ALEN] = {0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b};
>
> What does gd and bd mean?
Good and Bad :D
>
> > + struct stmmac_packet_attrs attr = { };
> > + int ret;
> > +
> > + if (stmmac_filter_check(priv))
> > + return -EOPNOTSUPP;
>
> and just return the error code from the call.
>
> > +
> > + ret = dev_mc_add(priv->dev, gd_addr);
> > + if (ret)
> > + return ret;
> > +
> > + attr.dst = gd_addr;
> > +
> > + /* Shall receive packet */
> > + ret = __stmmac_test_loopback(priv, &attr);
> > + if (ret)
> > + goto cleanup;
> > +
> > + attr.dst = bd_addr;
> > +
> > + /* Shall NOT receive packet */
> > + ret = __stmmac_test_loopback(priv, &attr);
> > + ret = !ret;
>
> What is this test testing? gd is a multicast, where as bd is not. I
> expect the hardware treats multicast different to unicast. So it would
> make more sense to test two different multicast addresses, one which
> has been added via dev_mc_addr, and one that has not?
Hmm, yeah makes sense. I will adjust.
>
> > +
> > +cleanup:
> > + dev_mc_del(priv->dev, gd_addr);
> > + return ret;
> > +}
> > +
> > +static int stmmac_test_pfilt(struct stmmac_priv *priv)
> > +{
> > + unsigned char gd_addr[ETH_ALEN] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06};
> > + unsigned char bd_addr[ETH_ALEN] = {0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b};
> > + struct stmmac_packet_attrs attr = { };
> > + int ret;
> > +
> > + if (stmmac_filter_check(priv))
> > + return -EOPNOTSUPP;
> > +
> > + ret = dev_uc_add(priv->dev, gd_addr);
> > + if (ret)
> > + return ret;
> > +
> > + attr.dst = gd_addr;
> > +
> > + /* Shall receive packet */
> > + ret = __stmmac_test_loopback(priv, &attr);
> > + if (ret)
> > + goto cleanup;
>
> gb is a multicast address. Does dev_uc_add() return an error? If it
> does not we should not expect it to actually work, since a multicast
> address should not match a unicast address?
It doesn't return an error and it does calls the set_filter callback in
netdev. I will adjust to use unicast address.
> You also seem to be missing a test for adding a unicast address via
> dev_uc_add() and receiving packets for that address, but not receiving
> multicast packets.
Hmm, what if interface was already configured to receive Multicast before
running the tests ?
>
> > +static const struct stmmac_test {
> > + char name[ETH_GSTRING_LEN];
> > + int lb;
> > + int (*fn)(struct stmmac_priv *priv);
> > +} stmmac_selftests[] = {
> > + {
> > + .name = "MAC Loopback ",
> > + .lb = STMMAC_LOOPBACK_MAC,
> > + .fn = stmmac_test_loopback,
>
> stmmac_test_mac_loopback might be a better name.
Ok.
Thanks for the review!
Thanks,
Jose Miguel Abreu
On Wed, May 08, 2019 at 09:51:00AM +0200, Jose Abreu wrote:
> [ Submitting with net-next closed for proper review and testing. ]
>
> This introduces selftests support in stmmac driver. We add 4 basic sanity
> checks and MAC loopback support for all cores within the driver. This way
> more tests can easily be added in the future and can be run in virtually
> any MAC/GMAC/QoS/XGMAC platform.
>
> Having this we can find regressions and missing features in the driver
> while at the same time we can check if the IP is correctly working.
>
> We have been using this for some time now and I do have more tests to
> submit in the feature. My experience is that although writing the tests
> adds more development time, the gain results are obvious.
>
> I let this feature optional within the driver under a Kconfig option.
>
> For this series the output result will be something like this
> (e.g. for dwmac1000):
> ----
> # ethtool -t eth0
> The test result is PASS
> The test extra info:
> 1. MAC Loopback 0
> 2. PHY Loopback -95
> 3. MMC Counters 0
> 4. EEE -95
> 5. Hash Filter MC 0
> 6. Perfect Filter UC 0
> 7. Flow Control 0
> ----
>
> (Error code -95 means EOPNOTSUPP in current HW).
>
Hello
I have started to patch dwmac_sun8i for using your patchset and get the following:
The test result is FAIL
The test extra info:
1. MAC Loopback 0
2. PHY Loopback -95
3. MMC Counters -95
4. EEE -95
5. Hash Filter MC 0
6. Perfect Filter UC 1
7. Flow Control -95
What means 1 for "Perfect Filter UC" ?
I have added my patch below
Regards
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -976,6 +976,18 @@ static void sun8i_dwmac_exit(struct platform_device *pdev, void *priv)
regulator_disable(gmac->regulator);
}
+static void sun8i_dwmac_set_mac_loopback(void __iomem *ioaddr, bool enable)
+{
+ u32 value = readl(ioaddr + EMAC_BASIC_CTL0);
+
+ if (enable)
+ value |= EMAC_LOOPBACK;
+ else
+ value &= ~EMAC_LOOPBACK;
+
+ writel(value, ioaddr + EMAC_BASIC_CTL0);
+}
+
static const struct stmmac_ops sun8i_dwmac_ops = {
.core_init = sun8i_dwmac_core_init,
.set_mac = sun8i_dwmac_set_mac,
@@ -985,6 +997,7 @@ static const struct stmmac_ops sun8i_dwmac_ops = {
.flow_ctrl = sun8i_dwmac_flow_ctrl,
.set_umac_addr = sun8i_dwmac_set_umac_addr,
.get_umac_addr = sun8i_dwmac_get_umac_addr,
+ .set_mac_loopback = sun8i_dwmac_set_mac_loopback,
};
static struct mac_device_info *sun8i_dwmac_setup(void *ppriv)
From: Corentin Labbe <[email protected]>
Date: Thu, May 09, 2019 at 10:04:16
> What means 1 for "Perfect Filter UC" ?
Thank you for the testing :)
1 means that either the expected packet was not received or that the
filter did not work.
For GMAC there is the need to set the HPF bit in order for the test to
pass. Do you have such bit in your HW ? It should be in EMAC_RX_FRM_FLT
register.
> I have added my patch below
Do you want me to add your patch to the series ? If you send me with
git-send-email I can apply it with your SoB.
Thanks,
Jose Miguel Abreu
> > You also seem to be missing a test for adding a unicast address via
> > dev_uc_add() and receiving packets for that address, but not receiving
> > multicast packets.
>
> Hmm, what if interface was already configured to receive Multicast before
> running the tests ?
The kernel keeps a list of unicast and multicast addresses, which have
been added to the filters. You could remove them all, do the test, and
then add them back. __dev_mc_unsync(), __dev_mc_sync() etc.
Andrew
From: Andrew Lunn <[email protected]>
Date: Thu, May 09, 2019 at 13:21:18
> > > You also seem to be missing a test for adding a unicast address via
> > > dev_uc_add() and receiving packets for that address, but not receiving
> > > multicast packets.
> >
> > Hmm, what if interface was already configured to receive Multicast before
> > running the tests ?
>
> The kernel keeps a list of unicast and multicast addresses, which have
> been added to the filters. You could remove them all, do the test, and
> then add them back. __dev_mc_unsync(), __dev_mc_sync() etc.
Thanks! I've implemented the "MC Filter" and "UC Filter" tests and due to
that I found another bug in the driver :D
Thanks,
Jose Miguel Abreu
From: Jose Abreu <[email protected]>
Date: Thu, May 09, 2019 at 09:17:03
> From: Andrew Lunn <[email protected]>
> Date: Wed, May 08, 2019 at 20:50:11
>
> > The normal operation is interrupted by the tests you carry out
> > here. But i don't see any code looking for ETH_TEST_FL_OFFLINE
>
> Ok will fix to only run in offline mode then.
>
> >
> > > (Error code -95 means EOPNOTSUPP in current HW).
> >
> > How deep do you have to go before you know about EOPNOTSUPP? It would
> > be better to not return the string and result at all. Or patch ethtool
> > to call strerror(3).
>
> When I looked at other drivers I saw that they return positive value (1)
> or zero so calling strerror in ethtool may not be ideal.
>
> I think its useful to let the user know if a given test is not supported
> in HW so maybe I can return 1 instead of EOPNOTSUPP ?
After thinking about this in more detail I now realize that returning 1
is not ideal because when a test fails it will also return 1. So if I do
it this way and more than one test fails then user won't know if the test
really failed or if it wasn't supported in the first time.
Any advice ?
Thanks,
Jose Miguel Abreu
On Thu, May 09, 2019 at 10:11:53AM +0000, Jose Abreu wrote:
> From: Corentin Labbe <[email protected]>
> Date: Thu, May 09, 2019 at 10:04:16
>
> > What means 1 for "Perfect Filter UC" ?
>
> Thank you for the testing :)
>
> 1 means that either the expected packet was not received or that the
> filter did not work.
>
> For GMAC there is the need to set the HPF bit in order for the test to
> pass. Do you have such bit in your HW ? It should be in EMAC_RX_FRM_FLT
> register.
The problem was missing IFF_UNICAST_FLT, so netcore put the device in promiscous when adding an unicast address.
Next step will be to enable Flow Control, but I will start a new thread for that.
>
> > I have added my patch below
>
> Do you want me to add your patch to the series ? If you send me with
> git-send-email I can apply it with your SoB.
>
I will do.
I will send the patch for IFF_UNICAST_FLT appart since it fixes a bug.
Regards