[LEDE-DEV] [PATCH RFC] ar8216: (try to) implement get_port_stats

Vittorio Gambaletta (VittGam) openwrt at vittgam.net
Tue Mar 21 17:25:28 PDT 2017


Hello,

TP-LINK, in their new TL-WR1043ND v4, for some reason wired the
ethernet LEDs not to the AR8337, but to the QCA956x SoC instead.

This means that the switch LED trigger needs to be used for all
of the ethernet LEDs (4 x LAN and 1 x WAN), like on the AR9341
when its internal switch is used.

But the ar8216/8327/8337 switch driver does not implement the
get_port_stats function, so the LEDs are lit but do not blink
when traffic passes through the ports.

(By the way, if I remember well, the stock firmware of this router
does not make those LEDs blink either...)

I've tried to write the needed function, as you can see in the
patch below.

The problem is that now a kworker kernel thread is constantly
consuming 12-15% of CPU. It stops when I change the trigger on
the LEDs to something else via sysfs. So, there is clearly
something wrong with it. But what?

The implementation for the AR9341 internal switch (in
target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_ar7240.c)
does it in a quite similar way (it actually polls ALL the switch
ports every time data for a single port is requested...), but
it uses 0% CPU instead.

What do you think?

Thank you!

Best regards,
Vittorio G

---

diff --git a/target/linux/generic/files/drivers/net/phy/ar8216.c b/target/linux/generic/files/drivers/net/phy/ar8216.c
index 37877d5..6b3246d 100644
--- a/target/linux/generic/files/drivers/net/phy/ar8216.c
+++ b/target/linux/generic/files/drivers/net/phy/ar8216.c
@@ -49,6 +49,13 @@ extern const struct ar8xxx_chip ar8337_chip;
 		.name = (_n),	\
 	}
 
+enum {
+	AR8216_MIB_TX_OFFSET = 29,
+	AR8216_MIB_RX_OFFSET = 14,
+	AR8236_MIB_TX_OFFSET = 31,
+	AR8236_MIB_RX_OFFSET = 15,
+};
+
 static const struct ar8xxx_mib_desc ar8216_mibs[] = {
 	MIB_DESC(1, AR8216_STATS_RXBROAD, "RxBroad"),
 	MIB_DESC(1, AR8216_STATS_RXPAUSE, "RxPause"),
@@ -394,6 +401,24 @@ ar8xxx_mib_flush(struct ar8xxx_priv *priv)
 	return ar8xxx_mib_op(priv, AR8216_MIB_FUNC_FLUSH);
 }
 
+static u64
+ar8xxx_mib_fetch_single_port_stat(struct ar8xxx_priv *priv,
+                                  const unsigned int base,
+                                  const struct ar8xxx_mib_desc *mib)
+{
+	u64 t;
+
+	t = ar8xxx_read(priv, base + mib->offset);
+	if (mib->size == 2) {
+		u64 hi;
+
+		hi = ar8xxx_read(priv, base + mib->offset + 4);
+		t |= hi << 32;
+	}
+
+	return t;
+}
+
 static void
 ar8xxx_mib_fetch_port_stat(struct ar8xxx_priv *priv, int port, bool flush)
 {
@@ -410,17 +435,7 @@ ar8xxx_mib_fetch_port_stat(struct ar8xxx_priv *priv, int port, bool flush)
 
 	mib_stats = &priv->mib_stats[port * priv->chip->num_mibs];
 	for (i = 0; i < priv->chip->num_mibs; i++) {
-		const struct ar8xxx_mib_desc *mib;
-		u64 t;
-
-		mib = &priv->chip->mib_decs[i];
-		t = ar8xxx_read(priv, base + mib->offset);
-		if (mib->size == 2) {
-			u64 hi;
-
-			hi = ar8xxx_read(priv, base + mib->offset + 4);
-			t |= hi << 32;
-		}
+		u64 t = ar8xxx_mib_fetch_single_port_stat(priv, base, &priv->chip->mib_decs[i]);
 
 		if (flush)
 			mib_stats[i] = 0;
@@ -1001,6 +1016,45 @@ ar8xxx_sw_get_port_link(struct switch_dev *dev, int port,
 	return 0;
 }
 
+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);
+	const struct ar8xxx_chip *chip = priv->chip;
+	unsigned int base, tx_offset, rx_offset;
+	u64 *mib_stats;
+
+	if (!ar8xxx_has_mib_counters(priv))
+		return -EOPNOTSUPP;
+
+	if (port >= dev->ports)
+		return -EINVAL;
+
+	mutex_lock(&priv->mib_lock);
+
+	base = chip->reg_port_stats_start + chip->reg_port_stats_length * port;
+	mib_stats = &priv->mib_stats[port * chip->num_mibs];
+
+	if (chip_is_ar8216(priv)) {
+		tx_offset = AR8216_MIB_TX_OFFSET;
+		rx_offset = AR8216_MIB_RX_OFFSET;
+	} else {
+		tx_offset = AR8236_MIB_TX_OFFSET;
+		rx_offset = AR8236_MIB_RX_OFFSET;
+	}
+
+	mib_stats[tx_offset] += ar8xxx_mib_fetch_single_port_stat(priv, base, &priv->chip->mib_decs[tx_offset]);
+	mib_stats[rx_offset] += ar8xxx_mib_fetch_single_port_stat(priv, base, &priv->chip->mib_decs[rx_offset]);
+
+	stats->tx_bytes = mib_stats[tx_offset];
+	stats->rx_bytes = mib_stats[rx_offset];
+
+	mutex_unlock(&priv->mib_lock);
+
+	return 0;
+}
+
 static int
 ar8xxx_sw_get_ports(struct switch_dev *dev, struct switch_val *val)
 {
@@ -1696,6 +1750,7 @@ 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,
+	.get_port_stats = ar8xxx_sw_get_port_stats,
 };
 
 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 d9508b9..e6c282a 100644
--- a/target/linux/generic/files/drivers/net/phy/ar8216.h
+++ b/target/linux/generic/files/drivers/net/phy/ar8216.h
@@ -538,6 +538,9 @@ int
 ar8xxx_sw_get_port_link(struct switch_dev *dev, int port,
 			struct switch_port_link *link);
 int
+ar8xxx_sw_get_port_stats(struct switch_dev *dev,
+                         int port, struct switch_port_stats *stats);
+int
 ar8xxx_sw_set_port_reset_mib(struct switch_dev *dev,
                              const struct switch_attr *attr,
                              struct switch_val *val);
diff --git a/target/linux/generic/files/drivers/net/phy/ar8327.c b/target/linux/generic/files/drivers/net/phy/ar8327.c
index 8e67f4b..9ca692d 100644
--- a/target/linux/generic/files/drivers/net/phy/ar8327.c
+++ b/target/linux/generic/files/drivers/net/phy/ar8327.c
@@ -1370,6 +1370,7 @@ 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,
+	.get_port_stats = ar8xxx_sw_get_port_stats,
 };
 
 const struct ar8xxx_chip ar8327_chip = {



More information about the Lede-dev mailing list