From: Thibaut VARENE <hacks@slashdirt.org>
Date: Tue, 12 Sep 2017 21:22:37 +0000 (+0200)
Subject: generic: drop support for get_port_stats() on ar8xxx
X-Git-Url: http://git.openwrt.org/?p=openwrt%2Fstaging%2Fwigyori.git;a=commitdiff_plain;h=eff3549c5883a9abc5dbff00c084cabbcfdf4437

generic: drop support for get_port_stats() on ar8xxx

The implementation is not efficient on ar8xxx switches. It triggers high
CPU load and degrades device performance.

The high CPU load has been traced down to the ar8xxx_reg_wait() call in
ar8xxx_mib_op(), which has to usleep_range() till the MIB busy flag set
by the request to update the MIB counter is cleared.

This commit removes the get_port_stats() code introduced in 4d8a66d and
leaves a note for future hacker's beware.

Fixes: FS#1004

Signed-off-by: Thibaut VARENE <hacks@slashdirt.org>
---

diff --git a/target/linux/generic/files/drivers/net/phy/ar8216.c b/target/linux/generic/files/drivers/net/phy/ar8216.c
index 7f3d5115ab..61811370de 100644
--- a/target/linux/generic/files/drivers/net/phy/ar8216.c
+++ b/target/linux/generic/files/drivers/net/phy/ar8216.c
@@ -49,12 +49,6 @@ extern const struct ar8xxx_chip ar8337_chip;
 		.name = (_n),	\
 	}
 
-#define AR8216_MIB_RXB_ID	14	/* RxGoodByte */
-#define AR8216_MIB_TXB_ID	29	/* TxByte */
-
-#define AR8236_MIB_RXB_ID	15	/* RxGoodByte */
-#define AR8236_MIB_TXB_ID	31	/* TxByte */
-
 static const struct ar8xxx_mib_desc ar8216_mibs[] = {
 	MIB_DESC(1, AR8216_STATS_RXBROAD, "RxBroad"),
 	MIB_DESC(1, AR8216_STATS_RXPAUSE, "RxPause"),
@@ -1587,56 +1581,6 @@ ar8xxx_sw_set_flush_port_arl_table(struct switch_dev *dev,
 	return ret;
 }
 
-int
-ar8xxx_sw_get_port_stats(struct switch_dev *dev, int port,
-			struct switch_port_stats *stats)
-{
-	struct ar8xxx_priv *priv = swdev_to_ar8xxx(dev);
-	u64 *mib_stats;
-	int ret;
-	int mib_txb_id, mib_rxb_id;
-
-	if (!ar8xxx_has_mib_counters(priv))
-		return -EOPNOTSUPP;
-
-	if (port >= dev->ports)
-		return -EINVAL;
-
-	switch (priv->chip_ver) {
-		case AR8XXX_VER_AR8216:
-			mib_txb_id = AR8216_MIB_TXB_ID;
-			mib_rxb_id = AR8216_MIB_RXB_ID;
-			break;
-		case AR8XXX_VER_AR8236:
-		case AR8XXX_VER_AR8316:
-		case AR8XXX_VER_AR8327:
-		case AR8XXX_VER_AR8337:
-			mib_txb_id = AR8236_MIB_TXB_ID;
-			mib_rxb_id = AR8236_MIB_RXB_ID;
-			break;
-		default:
-			return -EOPNOTSUPP;
-	}
-
-	mutex_lock(&priv->mib_lock);
-	ret = ar8xxx_mib_capture(priv);
-	if (ret)
-		goto unlock;
-
-	ar8xxx_mib_fetch_port_stat(priv, port, false);
-
-	mib_stats = &priv->mib_stats[port * priv->chip->num_mibs];
-
-	stats->tx_bytes = mib_stats[mib_txb_id];
-	stats->rx_bytes = mib_stats[mib_rxb_id];
-
-	ret = 0;
-
-unlock:
-	mutex_unlock(&priv->mib_lock);
-	return ret;
-}
-
 static const struct switch_attr ar8xxx_sw_attr_globals[] = {
 	{
 		.type = SWITCH_TYPE_INT,
@@ -1752,7 +1696,16 @@ static const struct switch_dev_ops ar8xxx_sw_ops = {
 	.apply_config = ar8xxx_sw_hw_apply,
 	.reset_switch = ar8xxx_sw_reset_switch,
 	.get_port_link = ar8xxx_sw_get_port_link,
+/* The following op is disabled as it hogs the CPU and degrades performance.
+   An implementation has been attempted in 4d8a66d but reading MIB data is slow
+   on ar8xxx switches.
+
+   The high CPU load has been traced down to the ar8xxx_reg_wait() call in
+   ar8xxx_mib_op(), which has to usleep_range() till the MIB busy flag set by
+   the request to update the MIB counter is cleared. */
+#if 0
 	.get_port_stats = ar8xxx_sw_get_port_stats,
+#endif
 };
 
 static const struct ar8xxx_chip ar8216_chip = {
diff --git a/target/linux/generic/files/drivers/net/phy/ar8216.h b/target/linux/generic/files/drivers/net/phy/ar8216.h
index 321eeb7923..d9508b9ff8 100644
--- a/target/linux/generic/files/drivers/net/phy/ar8216.h
+++ b/target/linux/generic/files/drivers/net/phy/ar8216.h
@@ -566,9 +566,6 @@ ar8xxx_sw_set_flush_port_arl_table(struct switch_dev *dev,
 				   const struct switch_attr *attr,
 				   struct switch_val *val);
 int
-ar8xxx_sw_get_port_stats(struct switch_dev *dev, int port,
-			struct switch_port_stats *stats);
-int
 ar8216_wait_bit(struct ar8xxx_priv *priv, int reg, u32 mask, u32 val);
 
 static inline struct ar8xxx_priv *
diff --git a/target/linux/generic/files/drivers/net/phy/ar8327.c b/target/linux/generic/files/drivers/net/phy/ar8327.c
index bae9744736..6ebd2e8aed 100644
--- a/target/linux/generic/files/drivers/net/phy/ar8327.c
+++ b/target/linux/generic/files/drivers/net/phy/ar8327.c
@@ -1411,7 +1411,16 @@ static const struct switch_dev_ops ar8327_sw_ops = {
 	.apply_config = ar8327_sw_hw_apply,
 	.reset_switch = ar8xxx_sw_reset_switch,
 	.get_port_link = ar8xxx_sw_get_port_link,
+/* The following op is disabled as it hogs the CPU and degrades performance.
+   An implementation has been attempted in 4d8a66d but reading MIB data is slow
+   on ar8xxx switches.
+
+   The high CPU load has been traced down to the ar8xxx_reg_wait() call in
+   ar8xxx_mib_op(), which has to usleep_range() till the MIB busy flag set by
+   the request to update the MIB counter is cleared. */
+#if 0
 	.get_port_stats = ar8xxx_sw_get_port_stats,
+#endif
 };
 
 const struct ar8xxx_chip ar8327_chip = {