[PATCHv2 net-next 0/3] Ethtool support for phy stats

[PATCHv2 net-next 0/3] Ethtool support for phy stats

[Andrew Lunn]

This patchset add ethtool support for reading statistics from the PHY.
The Marvell and Micrel Phys are then extended to report receiver
packet errors and idle errors.

v2:
  Fix linking when phylib is not enabled.

Andrew Lunn (3):
  ethtool: Add phy statistics
  phy: marvell: Add ethtool statistics counters
  phy: micrel: Add ethtool statistics counters

 drivers/net/phy/marvell.c    | 135 +++++++++++++++++++++++++++++++++++++++++++
 drivers/net/phy/micrel.c     |  96 ++++++++++++++++++++++++++++++
 drivers/net/phy/phy.c        |  35 +++++++++++
 include/linux/phy.h          |  11 +++-
 include/uapi/linux/ethtool.h |   3 +
 net/core/ethtool.c           |  57 +++++++++++++++++-
 6 files changed, 335 insertions(+), 2 deletions(-)

-- 
2.6.4

[PATCHv2 net-next 1/3] ethtool: Add phy statistics

[Andrew Lunn]

Ethernet PHYs can maintain statistics, for example errors while idle
and receive errors. Add an ethertool mechanism to retrieve these
statistics, using the same model as MAC statistics.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/phy.c        | 35 +++++++++++++++++++++++++++
 include/linux/phy.h          | 11 ++++++++-
 include/uapi/linux/ethtool.h |  3 +++
 net/core/ethtool.c           | 57 +++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 47cd306dbb3c..fc6a770adf40 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1262,3 +1262,38 @@ void phy_ethtool_get_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol)
 		phydev->drv->get_wol(phydev, wol);
 }
 EXPORT_SYMBOL(phy_ethtool_get_wol);
+
+int phy_get_sset_count(struct phy_device *phydev)
+{
+	int ret;
+
+	if (phydev->drv->get_sset_count &&
+	    phydev->drv->get_strings &&
+	    phydev->drv->get_stats) {
+		mutex_lock(&phydev->lock);
+		ret = phydev->drv->get_sset_count(phydev);
+		mutex_unlock(&phydev->lock);
+
+		return ret;
+	}
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(phy_get_sset_count);
+
+void phy_get_strings(struct phy_device *phydev, u8 *data)
+{
+	mutex_lock(&phydev->lock);
+	phydev->drv->get_strings(phydev, data);
+	mutex_unlock(&phydev->lock);
+}
+EXPORT_SYMBOL(phy_get_strings);
+
+void phy_get_stats(struct phy_device *phydev, struct ethtool_stats *stats,
+		   u64 *data)
+{
+	mutex_lock(&phydev->lock);
+	phydev->drv->get_stats(phydev, stats, data);
+	mutex_unlock(&phydev->lock);
+}
+EXPORT_SYMBOL(phy_get_stats);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 05fde31b6dc6..d457c6efbbd5 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -589,6 +589,12 @@ struct phy_driver {
 	int (*module_eeprom)(struct phy_device *dev,
 			     struct ethtool_eeprom *ee, u8 *data);
 
+	/* Get statistics from the phy using ethtool */
+	int (*get_sset_count)(struct phy_device *dev);
+	void (*get_strings)(struct phy_device *dev, u8 *data);
+	void (*get_stats)(struct phy_device *dev,
+			  struct ethtool_stats *stats, u64 *data);
+
 	struct device_driver driver;
 };
 #define to_phy_driver(d) container_of(d, struct phy_driver, driver)
@@ -816,7 +822,10 @@ int phy_ethtool_get_eee(struct phy_device *phydev, struct ethtool_eee *data);
 int phy_ethtool_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *wol);
 void phy_ethtool_get_wol(struct phy_device *phydev,
 			 struct ethtool_wolinfo *wol);
-
+int phy_get_sset_count(struct phy_device *phydev);
+void phy_get_strings(struct phy_device *phydev, u8 *data);
+void phy_get_stats(struct phy_device *phydev, struct ethtool_stats *stats,
+		   u64 *data);
 int __init mdio_bus_init(void);
 void mdio_bus_exit(void);
 
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index cd1629170103..57fa39005e79 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -542,6 +542,7 @@ struct ethtool_pauseparam {
  *	now deprecated
  * @ETH_SS_FEATURES: Device feature names
  * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names
+ * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS
  */
 enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
@@ -551,6 +552,7 @@ enum ethtool_stringset {
 	ETH_SS_FEATURES,
 	ETH_SS_RSS_HASH_FUNCS,
 	ETH_SS_TUNABLES,
+	ETH_SS_PHY_STATS,
 };
 
 /**
@@ -1225,6 +1227,7 @@ enum ethtool_sfeatures_retval_bits {
 #define ETHTOOL_SRSSH		0x00000047 /* Set RX flow hash configuration */
 #define ETHTOOL_GTUNABLE	0x00000048 /* Get tunable configuration */
 #define ETHTOOL_STUNABLE	0x00000049 /* Set tunable configuration */
+#define ETHTOOL_GPHYSTATS	0x0000004a /* get PHY-specific statistics */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 09948a726347..ee88bc06ccaf 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -204,6 +204,13 @@ static int __ethtool_get_sset_count(struct net_device *dev, int sset)
 	if (sset == ETH_SS_TUNABLES)
 		return ARRAY_SIZE(tunable_strings);
 
+	if (sset == ETH_SS_PHY_STATS) {
+		if (IS_ENABLED(CONFIG_PHYLIB) && dev->phydev)
+			return phy_get_sset_count(dev->phydev);
+		else
+			return -EOPNOTSUPP;
+	}
+
 	if (ops->get_sset_count && ops->get_strings)
 		return ops->get_sset_count(dev, sset);
 	else
@@ -223,7 +230,12 @@ static void __ethtool_get_strings(struct net_device *dev,
 		       sizeof(rss_hash_func_strings));
 	else if (stringset == ETH_SS_TUNABLES)
 		memcpy(data, tunable_strings, sizeof(tunable_strings));
-	else
+	else if (stringset == ETH_SS_PHY_STATS) {
+		if (IS_ENABLED(CONFIG_PHYLIB) && dev->phydev)
+			phy_get_strings(dev->phydev, data);
+		else
+			return;
+	} else
 		/* ops->get_strings is valid because checked earlier */
 		ops->get_strings(dev, stringset, data);
 }
@@ -1401,6 +1413,45 @@ static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
 	return ret;
 }
 
+static int ethtool_get_phy_stats(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_stats stats;
+	struct phy_device *phydev = dev->phydev;
+	u64 *data;
+	int ret, n_stats;
+
+	if (!IS_ENABLED(CONFIG_PHYLIB) || !phydev)
+		return -EOPNOTSUPP;
+
+	n_stats = phy_get_sset_count(phydev);
+
+	if (n_stats < 0)
+		return n_stats;
+	WARN_ON(n_stats == 0);
+
+	if (copy_from_user(&stats, useraddr, sizeof(stats)))
+		return -EFAULT;
+
+	stats.n_stats = n_stats;
+	data = kmalloc_array(n_stats, sizeof(u64), GFP_USER);
+	if (!data)
+		return -ENOMEM;
+
+	phy_get_stats(phydev, &stats, data);
+
+	ret = -EFAULT;
+	if (copy_to_user(useraddr, &stats, sizeof(stats)))
+		goto out;
+	useraddr += sizeof(stats);
+	if (copy_to_user(useraddr, data, stats.n_stats * sizeof(u64)))
+		goto out;
+	ret = 0;
+
+ out:
+	kfree(data);
+	return ret;
+}
+
 static int ethtool_get_perm_addr(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_perm_addr epaddr;
@@ -1779,6 +1830,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GSSET_INFO:
 	case ETHTOOL_GSTRINGS:
 	case ETHTOOL_GSTATS:
+	case ETHTOOL_GPHYSTATS:
 	case ETHTOOL_GTSO:
 	case ETHTOOL_GPERMADDR:
 	case ETHTOOL_GUFO:
@@ -1991,6 +2043,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_STUNABLE:
 		rc = ethtool_set_tunable(dev, useraddr);
 		break;
+	case ETHTOOL_GPHYSTATS:
+		rc = ethtool_get_phy_stats(dev, useraddr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}
-- 
2.6.4

[PATCHv2 net-next 2/3] phy: marvell: Add ethtool statistics counters

[Andrew Lunn]

The PHY counters receiver errors and errors while idle.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/marvell.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 135 insertions(+)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 0240552b50f3..50b5eac75854 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -137,6 +137,22 @@ MODULE_DESCRIPTION("Marvell PHY driver");
 MODULE_AUTHOR("Andy Fleming");
 MODULE_LICENSE("GPL");
 
+struct marvell_hw_stat {
+	const char *string;
+	u8 page;
+	u8 reg;
+	u8 bits;
+};
+
+static struct marvell_hw_stat marvell_hw_stats[] = {
+	{ "phy_receive_errors", 0, 21, 16},
+	{ "phy_idle_errors", 0, 10, 8 },
+};
+
+struct marvell_priv {
+	u64 stats[ARRAY_SIZE(marvell_hw_stats)];
+};
+
 static int marvell_ack_interrupt(struct phy_device *phydev)
 {
 	int err;
@@ -986,12 +1002,80 @@ static int m88e1318_set_wol(struct phy_device *phydev, struct ethtool_wolinfo *w
 	return 0;
 }
 
+static int marvell_get_sset_count(struct phy_device *phydev)
+{
+	return ARRAY_SIZE(marvell_hw_stats);
+}
+
+static void marvell_get_strings(struct phy_device *phydev, u8 *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(marvell_hw_stats); i++) {
+		memcpy(data + i * ETH_GSTRING_LEN,
+		       marvell_hw_stats[i].string, ETH_GSTRING_LEN);
+	}
+}
+
+#ifndef UINT64_MAX
+#define UINT64_MAX              (u64)(~((u64)0))
+#endif
+static u64 marvell_get_stat(struct phy_device *phydev, int i)
+{
+	struct marvell_hw_stat stat = marvell_hw_stats[i];
+	struct marvell_priv *priv = phydev->priv;
+	int err, oldpage;
+	u64 val;
+
+	oldpage = phy_read(phydev, MII_MARVELL_PHY_PAGE);
+	err = phy_write(phydev, MII_MARVELL_PHY_PAGE,
+			stat.page);
+	if (err < 0)
+		return UINT64_MAX;
+
+	val = phy_read(phydev, stat.reg);
+	if (val < 0) {
+		val = UINT64_MAX;
+	} else {
+		val = val & ((1 << stat.bits) - 1);
+		priv->stats[i] += val;
+		val = priv->stats[i];
+	}
+
+	phy_write(phydev, MII_MARVELL_PHY_PAGE, oldpage);
+
+	return val;
+}
+
+static void marvell_get_stats(struct phy_device *phydev,
+			      struct ethtool_stats *stats, u64 *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(marvell_hw_stats); i++)
+		data[i] = marvell_get_stat(phydev, i);
+}
+
+static int marvell_probe(struct phy_device *phydev)
+{
+	struct marvell_priv *priv;
+
+	priv = devm_kzalloc(&phydev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	return 0;
+}
+
 static struct phy_driver marvell_drivers[] = {
 	{
 		.phy_id = MARVELL_PHY_ID_88E1101,
 		.phy_id_mask = MARVELL_PHY_ID_MASK,
 		.name = "Marvell 88E1101",
 		.features = PHY_GBIT_FEATURES,
+		.probe = marvell_probe,
 		.flags = PHY_HAS_INTERRUPT,
 		.config_aneg = &marvell_config_aneg,
 		.read_status = &genphy_read_status,
@@ -999,6 +1083,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1007,6 +1094,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1112",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &m88e1111_config_init,
 		.config_aneg = &marvell_config_aneg,
 		.read_status = &genphy_read_status,
@@ -1014,6 +1102,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1022,6 +1113,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1111",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &m88e1111_config_init,
 		.config_aneg = &marvell_config_aneg,
 		.read_status = &marvell_read_status,
@@ -1029,6 +1121,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1037,6 +1132,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1118",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &m88e1118_config_init,
 		.config_aneg = &m88e1118_config_aneg,
 		.read_status = &genphy_read_status,
@@ -1044,6 +1140,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = {.owner = THIS_MODULE,},
 	},
 	{
@@ -1052,6 +1151,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1121R",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_aneg = &m88e1121_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,
@@ -1059,6 +1159,9 @@ static struct phy_driver marvell_drivers[] = {
 		.did_interrupt = &m88e1121_did_interrupt,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1067,6 +1170,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1318S",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_aneg = &m88e1318_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,
@@ -1076,6 +1180,9 @@ static struct phy_driver marvell_drivers[] = {
 		.set_wol = &m88e1318_set_wol,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1084,6 +1191,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1145",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &m88e1145_config_init,
 		.config_aneg = &marvell_config_aneg,
 		.read_status = &genphy_read_status,
@@ -1091,6 +1199,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1099,6 +1210,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1149R",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &m88e1149_config_init,
 		.config_aneg = &m88e1118_config_aneg,
 		.read_status = &genphy_read_status,
@@ -1106,6 +1218,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1114,6 +1229,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1240",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &m88e1111_config_init,
 		.config_aneg = &marvell_config_aneg,
 		.read_status = &genphy_read_status,
@@ -1121,6 +1237,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1129,6 +1248,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1116R",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_init = &m88e1116r_config_init,
 		.config_aneg = &genphy_config_aneg,
 		.read_status = &genphy_read_status,
@@ -1136,6 +1256,9 @@ static struct phy_driver marvell_drivers[] = {
 		.config_intr = &marvell_config_intr,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1144,6 +1267,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1510",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,
@@ -1151,6 +1275,9 @@ static struct phy_driver marvell_drivers[] = {
 		.did_interrupt = &m88e1121_did_interrupt,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1159,6 +1286,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E1540",
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,
@@ -1166,6 +1294,9 @@ static struct phy_driver marvell_drivers[] = {
 		.did_interrupt = &m88e1121_did_interrupt,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 	{
@@ -1174,6 +1305,7 @@ static struct phy_driver marvell_drivers[] = {
 		.name = "Marvell 88E3016",
 		.features = PHY_BASIC_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
+		.probe = marvell_probe,
 		.config_aneg = &genphy_config_aneg,
 		.config_init = &m88e3016_config_init,
 		.aneg_done = &marvell_aneg_done,
@@ -1183,6 +1315,9 @@ static struct phy_driver marvell_drivers[] = {
 		.did_interrupt = &m88e1121_did_interrupt,
 		.resume = &genphy_resume,
 		.suspend = &genphy_suspend,
+		.get_sset_count = marvell_get_sset_count,
+		.get_strings = marvell_get_strings,
+		.get_stats = marvell_get_stats,
 		.driver = { .owner = THIS_MODULE },
 	},
 };
-- 
2.6.4

[PATCHv2 net-next 3/3] phy: micrel: Add ethtool statistics counters

[Andrew Lunn]

The PHY counters receiver errors and errors while idle.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/micrel.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index e13ad6cdcc22..1a6048a8c29d 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -73,6 +73,17 @@
 
 #define PS_TO_REG				200
 
+struct kszphy_hw_stat {
+	const char *string;
+	u8 reg;
+	u8 bits;
+};
+
+static struct kszphy_hw_stat kszphy_hw_stats[] = {
+	{ "phy_receive_errors", 21, 16},
+	{ "phy_idle_errors", 10, 8 },
+};
+
 struct kszphy_type {
 	u32 led_mode_reg;
 	u16 interrupt_level_mask;
@@ -86,6 +97,7 @@ struct kszphy_priv {
 	int led_mode;
 	bool rmii_ref_clk_sel;
 	bool rmii_ref_clk_sel_val;
+	u64 stats[ARRAY_SIZE(kszphy_hw_stats)];
 };
 
 static const struct kszphy_type ksz8021_type = {
@@ -569,6 +581,51 @@ ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
 {
 }
 
+static int kszphy_get_sset_count(struct phy_device *phydev)
+{
+	return ARRAY_SIZE(kszphy_hw_stats);
+}
+
+static void kszphy_get_strings(struct phy_device *phydev, u8 *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(kszphy_hw_stats); i++) {
+		memcpy(data + i * ETH_GSTRING_LEN,
+		       kszphy_hw_stats[i].string, ETH_GSTRING_LEN);
+	}
+}
+
+#ifndef UINT64_MAX
+#define UINT64_MAX              (u64)(~((u64)0))
+#endif
+static u64 kszphy_get_stat(struct phy_device *phydev, int i)
+{
+	struct kszphy_hw_stat stat = kszphy_hw_stats[i];
+	struct kszphy_priv *priv = phydev->priv;
+	u64 val;
+
+	val = phy_read(phydev, stat.reg);
+	if (val < 0) {
+		val = UINT64_MAX;
+	} else {
+		val = val & ((1 << stat.bits) - 1);
+		priv->stats[i] += val;
+		val = priv->stats[i];
+	}
+
+	return val;
+}
+
+static void kszphy_get_stats(struct phy_device *phydev,
+			     struct ethtool_stats *stats, u64 *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(kszphy_hw_stats); i++)
+		data[i] = kszphy_get_stat(phydev, i);
+}
+
 static int kszphy_probe(struct phy_device *phydev)
 {
 	const struct kszphy_type *type = phydev->drv->driver_data;
@@ -642,6 +699,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -659,6 +719,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -676,6 +739,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -693,6 +759,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -710,6 +779,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -727,6 +799,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -743,6 +818,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -759,6 +837,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -773,6 +854,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE,},
@@ -788,6 +872,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.read_mmd_indirect = ksz9021_rd_mmd_phyreg,
@@ -805,6 +892,9 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= ksz9031_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE, },
@@ -817,6 +907,9 @@ static struct phy_driver ksphy_driver[] = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= ksz8873mll_config_aneg,
 	.read_status	= ksz8873mll_read_status,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE, },
@@ -829,6 +922,9 @@ static struct phy_driver ksphy_driver[] = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
+	.get_sset_count = kszphy_get_sset_count,
+	.get_strings	= kszphy_get_strings,
+	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 	.driver		= { .owner = THIS_MODULE, },
-- 
2.6.4

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[Florian Fainelli]

On December 27, 2015 3:58:25 AM PST, Andrew Lunn <andrew@lunn.ch> wrote:
>This patchset add ethtool support for reading statistics from the PHY.
>The Marvell and Micrel Phys are then extended to report receiver
>packet errors and idle errors.

Reviewed-by: Florian Fainelli <f.faibelli@gmail.com>

This is very useful, thanks! I like that you have introduced a separate interface rather than forcing the use of the existing get statistics method and then overlaying PHY stats to the existing MAC stats, this is just cleaner.

>
>v2:
>  Fix linking when phylib is not enabled.
>
>Andrew Lunn (3):
>  ethtool: Add phy statistics
>  phy: marvell: Add ethtool statistics counters
>  phy: micrel: Add ethtool statistics counters
>
>drivers/net/phy/marvell.c    | 135
>+++++++++++++++++++++++++++++++++++++++++++
> drivers/net/phy/micrel.c     |  96 ++++++++++++++++++++++++++++++
> drivers/net/phy/phy.c        |  35 +++++++++++
> include/linux/phy.h          |  11 +++-
> include/uapi/linux/ethtool.h |   3 +
> net/core/ethtool.c           |  57 +++++++++++++++++-
> 6 files changed, 335 insertions(+), 2 deletions(-)


-- 
Florian

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[Andrew Lunn]

On Sun, Dec 27, 2015 at 12:07:15PM -0800, Florian Fainelli wrote:
> On December 27, 2015 3:58:25 AM PST, Andrew Lunn <andrew@lunn.ch> wrote:
> >This patchset add ethtool support for reading statistics from the PHY.
> >The Marvell and Micrel Phys are then extended to report receiver
> >packet errors and idle errors.
> 
> Reviewed-by: Florian Fainelli <f.faibelli@gmail.com>
> 

> This is very useful, thanks! I like that you have introduced a
> separate interface rather than forcing the use of the existing get
> statistics method and then overlaying PHY stats to the existing MAC
> stats, this is just cleaner.

I started out by extending the MAC stats, but the code was ugly
dealing with the different cases and MAC and PHY statistics. This way
is a lot cleaner.

   Andrew

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Sun, 27 Dec 2015 12:58:25 +0100

> This patchset add ethtool support for reading statistics from the PHY.
> The Marvell and Micrel Phys are then extended to report receiver
> packet errors and idle errors.
> 
> v2:
>   Fix linking when phylib is not enabled.

Series applied, thanks Andrew.

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Mon, 28 Dec 2015 00:32:43 -0500 (EST)

> From: Andrew Lunn <andrew@lunn.ch>
> Date: Sun, 27 Dec 2015 12:58:25 +0100
> 
>> This patchset add ethtool support for reading statistics from the PHY.
>> The Marvell and Micrel Phys are then extended to report receiver
>> packet errors and idle errors.
>> 
>> v2:
>>   Fix linking when phylib is not enabled.
> 
> Series applied, thanks Andrew.

I have to revert this.

You can't call into PHY library code from net/core/ethtool.c,
because PHY lib can be modular.

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[Andrew Lunn]

On Mon, Dec 28, 2015 at 12:44:27AM -0500, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 28 Dec 2015 00:32:43 -0500 (EST)
> 
> > From: Andrew Lunn <andrew@lunn.ch>
> > Date: Sun, 27 Dec 2015 12:58:25 +0100
> > 
> >> This patchset add ethtool support for reading statistics from the PHY.
> >> The Marvell and Micrel Phys are then extended to report receiver
> >> packet errors and idle errors.
> >> 
> >> v2:
> >>   Fix linking when phylib is not enabled.
> > 
> > Series applied, thanks Andrew.
> 
> I have to revert this.
> 
> You can't call into PHY library code from net/core/ethtool.c,
> because PHY lib can be modular.

O.K.

How about we change PHYLIB from a tristate to a bool?

Thanks
	Andrew

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[Florian Fainelli]

On December 28, 2015 12:58:25 AM PST, Andrew Lunn <andrew@lunn.ch> wrote:
>On Mon, Dec 28, 2015 at 12:44:27AM -0500, David Miller wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Mon, 28 Dec 2015 00:32:43 -0500 (EST)
>> 
>> > From: Andrew Lunn <andrew@lunn.ch>
>> > Date: Sun, 27 Dec 2015 12:58:25 +0100
>> > 
>> >> This patchset add ethtool support for reading statistics from the
>PHY.
>> >> The Marvell and Micrel Phys are then extended to report receiver
>> >> packet errors and idle errors.
>> >> 
>> >> v2:
>> >>   Fix linking when phylib is not enabled.
>> > 
>> > Series applied, thanks Andrew.
>> 
>> I have to revert this.
>> 
>> You can't call into PHY library code from net/core/ethtool.c,
>> because PHY lib can be modular.
>
>O.K.
>
>How about we change PHYLIB from a tristate to a bool?

Could we make this in a two layer design instead? Core ethtool changes call into getter functions which are implemented by the netdev driver, if the netdev driver happens to use PHYLIB, calls into the PHYLIB helpers with its phydev? That would also allow a network driver already supporting PHY stats to report them through your new interface.

-- 
Florian

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[David Miller]

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Mon, 28 Dec 2015 09:27:25 -0800

> On December 28, 2015 12:58:25 AM PST, Andrew Lunn <andrew@lunn.ch> wrote:
>>On Mon, Dec 28, 2015 at 12:44:27AM -0500, David Miller wrote:
>>> From: David Miller <davem@davemloft.net>
>>> Date: Mon, 28 Dec 2015 00:32:43 -0500 (EST)
>>> 
>>> > From: Andrew Lunn <andrew@lunn.ch>
>>> > Date: Sun, 27 Dec 2015 12:58:25 +0100
>>> > 
>>> >> This patchset add ethtool support for reading statistics from the
>>PHY.
>>> >> The Marvell and Micrel Phys are then extended to report receiver
>>> >> packet errors and idle errors.
>>> >> 
>>> >> v2:
>>> >>   Fix linking when phylib is not enabled.
>>> > 
>>> > Series applied, thanks Andrew.
>>> 
>>> I have to revert this.
>>> 
>>> You can't call into PHY library code from net/core/ethtool.c,
>>> because PHY lib can be modular.
>>
>>O.K.
>>
>>How about we change PHYLIB from a tristate to a bool?
> 
> Could we make this in a two layer design instead? Core ethtool changes call into getter functions which are implemented by the netdev driver, if the netdev driver happens to use PHYLIB, calls into the PHYLIB helpers with its phydev? That would also allow a network driver already supporting PHY stats to report them through your new interface.

Or just put the helpers in net/core/ethtool.c

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[Florian Fainelli]

On December 28, 2015 12:02:13 PM PST, David Miller <davem@davemloft.net> wrote:
>From: Florian Fainelli <f.fainelli@gmail.com>
>Date: Mon, 28 Dec 2015 09:27:25 -0800
>
>> On December 28, 2015 12:58:25 AM PST, Andrew Lunn <andrew@lunn.ch>
>wrote:
>>>On Mon, Dec 28, 2015 at 12:44:27AM -0500, David Miller wrote:
>>>> From: David Miller <davem@davemloft.net>
>>>> Date: Mon, 28 Dec 2015 00:32:43 -0500 (EST)
>>>> 
>>>> > From: Andrew Lunn <andrew@lunn.ch>
>>>> > Date: Sun, 27 Dec 2015 12:58:25 +0100
>>>> > 
>>>> >> This patchset add ethtool support for reading statistics from
>the
>>>PHY.
>>>> >> The Marvell and Micrel Phys are then extended to report receiver
>>>> >> packet errors and idle errors.
>>>> >> 
>>>> >> v2:
>>>> >>   Fix linking when phylib is not enabled.
>>>> > 
>>>> > Series applied, thanks Andrew.
>>>> 
>>>> I have to revert this.
>>>> 
>>>> You can't call into PHY library code from net/core/ethtool.c,
>>>> because PHY lib can be modular.
>>>
>>>O.K.
>>>
>>>How about we change PHYLIB from a tristate to a bool?
>> 
>> Could we make this in a two layer design instead? Core ethtool
>changes call into getter functions which are implemented by the netdev
>driver, if the netdev driver happens to use PHYLIB, calls into the
>PHYLIB helpers with its phydev? That would also allow a network driver
>already supporting PHY stats to report them through your new interface.
>
>Or just put the helpers in net/core/ethtool.c

That would work, Intel drivers could benefit from that interface if it is not too phydev centric. We would have to know if the netdev uses phylib, but that's already the case for things like WoL and MII ioctl.


-- 
Florian

Re: [PATCHv2 net-next 0/3] Ethtool support for phy stats

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Mon, 28 Dec 2015 09:58:25 +0100

> On Mon, Dec 28, 2015 at 12:44:27AM -0500, David Miller wrote:
>> From: David Miller <davem@davemloft.net>
>> Date: Mon, 28 Dec 2015 00:32:43 -0500 (EST)
>> 
>> > From: Andrew Lunn <andrew@lunn.ch>
>> > Date: Sun, 27 Dec 2015 12:58:25 +0100
>> > 
>> >> This patchset add ethtool support for reading statistics from the PHY.
>> >> The Marvell and Micrel Phys are then extended to report receiver
>> >> packet errors and idle errors.
>> >> 
>> >> v2:
>> >>   Fix linking when phylib is not enabled.
>> > 
>> > Series applied, thanks Andrew.
>> 
>> I have to revert this.
>> 
>> You can't call into PHY library code from net/core/ethtool.c,
>> because PHY lib can be modular.
> 
> O.K.
> 
> How about we change PHYLIB from a tristate to a bool?

Beforehand it could be modular just fine, and thus save space
for people who do not use it.