[PATCH net-next 0/4] net: dsa: mv88e6xxx: Add "eth-mac" and "rmon" counter group support

[PATCH net-next 0/4] net: dsa: mv88e6xxx: Add "eth-mac" and "rmon" counter group support

[Tobias Waldekranz]

The majority of the changes (1/4) are about refactoring the existing
ethtool statistics support to make it possible to read individual
counters, rather than the whole set.

2/4 tries to collect all information about a stat in a single place
using a mapper macro, which is then used to generate the original list
of stats, along with a matching enum. checkpatch is less than amused
with this construct, but prior art exists (__BPF_FUNC_MAPPER in
include/uapi/linux/bpf.h, for example). Is there a better way forward?

With that in place, adding the actual counter groups is pretty
straight forward (3-4/4).

Tobias Waldekranz (4):
  net: dsa: mv88e6xxx: Create API to read a single stat counter
  net: dsa: mv88e6xxx: Give each hw stat an ID
  net: dsa: mv88e6xxx: Add "eth-mac" counter group support
  net: dsa: mv88e6xxx: Add "rmon" counter group support

 drivers/net/dsa/mv88e6xxx/chip.c | 405 +++++++++++++++++++++----------
 drivers/net/dsa/mv88e6xxx/chip.h |  27 ++-
 2 files changed, 288 insertions(+), 144 deletions(-)

-- 
2.34.1

[PATCH net-next 1/4] net: dsa: mv88e6xxx: Create API to read a single stat counter

[Tobias Waldekranz]

This change contains no functional change. We simply push the hardware
specific stats logic to a function reading a single counter, rather
than the whole set.

This is a preparatory change for the upcoming standard ethtool
statistics support (i.e. "eth-mac", "eth-ctrl" etc.).

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 160 ++++++++++++++++++-------------
 drivers/net/dsa/mv88e6xxx/chip.h |  27 +++---
 2 files changed, 105 insertions(+), 82 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 42b1acaca33a..ffd81174bda3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1000,7 +1000,7 @@ static struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
 };
 
 static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
-					    struct mv88e6xxx_hw_stat *s,
+					    const struct mv88e6xxx_hw_stat *s,
 					    int port, u16 bank1_select,
 					    u16 histogram)
 {
@@ -1183,59 +1183,82 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds, int port, int sset)
 	return count;
 }
 
-static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
-				     uint64_t *data, int types,
-				     u16 bank1_select, u16 histogram)
+static int mv88e6095_stats_get_stat(struct mv88e6xxx_chip *chip, int port,
+				    const struct mv88e6xxx_hw_stat *stat,
+				    uint64_t *data)
 {
-	struct mv88e6xxx_hw_stat *stat;
-	int i, j;
+	if (!(stat->type & (STATS_TYPE_BANK0 | STATS_TYPE_PORT)))
+		return 0;
 
-	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
-		stat = &mv88e6xxx_hw_stats[i];
-		if (stat->type & types) {
-			mv88e6xxx_reg_lock(chip);
-			data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port,
-							      bank1_select,
-							      histogram);
-			mv88e6xxx_reg_unlock(chip);
+	*data = _mv88e6xxx_get_ethtool_stat(chip, stat, port, 0,
+					    MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
+	return 1;
+}
 
-			j++;
-		}
-	}
-	return j;
+static int mv88e6250_stats_get_stat(struct mv88e6xxx_chip *chip, int port,
+				    const struct mv88e6xxx_hw_stat *stat,
+				    uint64_t *data)
+{
+	if (!(stat->type & STATS_TYPE_BANK0))
+		return 0;
+
+	*data = _mv88e6xxx_get_ethtool_stat(chip, stat, port, 0,
+					    MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
+	return 1;
 }
 
-static int mv88e6095_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
-				     uint64_t *data)
+static int mv88e6320_stats_get_stat(struct mv88e6xxx_chip *chip, int port,
+				    const struct mv88e6xxx_hw_stat *stat,
+				    uint64_t *data)
 {
-	return mv88e6xxx_stats_get_stats(chip, port, data,
-					 STATS_TYPE_BANK0 | STATS_TYPE_PORT,
-					 0, MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
+	if (!(stat->type & (STATS_TYPE_BANK0 | STATS_TYPE_BANK1)))
+		return 0;
+
+	*data = _mv88e6xxx_get_ethtool_stat(chip, stat, port,
+					    MV88E6XXX_G1_STATS_OP_BANK_1_BIT_9,
+					    MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
+	return 1;
 }
 
-static int mv88e6250_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
-				     uint64_t *data)
+static int mv88e6390_stats_get_stat(struct mv88e6xxx_chip *chip, int port,
+				    const struct mv88e6xxx_hw_stat *stat,
+				    uint64_t *data)
 {
-	return mv88e6xxx_stats_get_stats(chip, port, data, STATS_TYPE_BANK0,
-					 0, MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
+	if (!(stat->type & (STATS_TYPE_BANK0 | STATS_TYPE_BANK1)))
+		return 0;
+
+	*data = _mv88e6xxx_get_ethtool_stat(chip, stat, port,
+					    MV88E6XXX_G1_STATS_OP_BANK_1_BIT_10,
+					    0);
+	return 1;
 }
 
-static int mv88e6320_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
-				     uint64_t *data)
+static int mv88e6xxx_stats_get_stat(struct mv88e6xxx_chip *chip, int port,
+				    const struct mv88e6xxx_hw_stat *stat,
+				    uint64_t *data)
 {
-	return mv88e6xxx_stats_get_stats(chip, port, data,
-					 STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
-					 MV88E6XXX_G1_STATS_OP_BANK_1_BIT_9,
-					 MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
+	int ret = 0;
+
+	if (chip->info->ops->stats_get_stat) {
+		mv88e6xxx_reg_lock(chip);
+		ret = chip->info->ops->stats_get_stat(chip, port, stat, data);
+		mv88e6xxx_reg_unlock(chip);
+	}
+
+	return ret;
 }
 
-static int mv88e6390_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
+static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
 				     uint64_t *data)
 {
-	return mv88e6xxx_stats_get_stats(chip, port, data,
-					 STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
-					 MV88E6XXX_G1_STATS_OP_BANK_1_BIT_10,
-					 0);
+	struct mv88e6xxx_hw_stat *stat;
+	int i, j;
+
+	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
+		stat = &mv88e6xxx_hw_stats[i];
+		j += mv88e6xxx_stats_get_stat(chip, port, stat, &data[j]);
+	}
+	return j;
 }
 
 static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port,
@@ -1251,10 +1274,9 @@ static void mv88e6xxx_atu_vtu_get_stats(struct mv88e6xxx_chip *chip, int port,
 static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
 				uint64_t *data)
 {
-	int count = 0;
+	int count;
 
-	if (chip->info->ops->stats_get_stats)
-		count = chip->info->ops->stats_get_stats(chip, port, data);
+	count = mv88e6xxx_stats_get_stats(chip, port, data);
 
 	mv88e6xxx_reg_lock(chip);
 	if (chip->info->ops->serdes_get_stats) {
@@ -3973,7 +3995,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4011,7 +4033,7 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.mgmt_rsvd2cpu = mv88e6185_g2_mgmt_rsvd2cpu,
 	.ppu_enable = mv88e6185_g1_ppu_enable,
 	.ppu_disable = mv88e6185_g1_ppu_disable,
@@ -4052,7 +4074,7 @@ static const struct mv88e6xxx_ops mv88e6097_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4094,7 +4116,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4137,7 +4159,7 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4186,7 +4208,7 @@ static const struct mv88e6xxx_ops mv88e6141_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -4241,7 +4263,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4279,7 +4301,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4327,7 +4349,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4376,7 +4398,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4427,7 +4449,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4476,7 +4498,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4521,7 +4543,7 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4570,7 +4592,7 @@ static const struct mv88e6xxx_ops mv88e6190_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -4628,7 +4650,7 @@ static const struct mv88e6xxx_ops mv88e6190x_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -4684,7 +4706,7 @@ static const struct mv88e6xxx_ops mv88e6191_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -4743,7 +4765,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -4796,7 +4818,7 @@ static const struct mv88e6xxx_ops mv88e6250_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6250_stats_get_sset_count,
 	.stats_get_strings = mv88e6250_stats_get_strings,
-	.stats_get_stats = mv88e6250_stats_get_stats,
+	.stats_get_stat = mv88e6250_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6250_watchdog_ops,
@@ -4843,7 +4865,7 @@ static const struct mv88e6xxx_ops mv88e6290_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -4902,7 +4924,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6320_stats_get_stats,
+	.stats_get_stat = mv88e6320_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -4949,7 +4971,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6320_stats_get_stats,
+	.stats_get_stat = mv88e6320_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -4998,7 +5020,7 @@ static const struct mv88e6xxx_ops mv88e6341_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -5056,7 +5078,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -5102,7 +5124,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -5153,7 +5175,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.stats_set_histogram = mv88e6095_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6095_stats_get_sset_count,
 	.stats_get_strings = mv88e6095_stats_get_strings,
-	.stats_get_stats = mv88e6095_stats_get_stats,
+	.stats_get_stat = mv88e6095_stats_get_stat,
 	.set_cpu_port = mv88e6095_g1_set_cpu_port,
 	.set_egress_port = mv88e6095_g1_set_egress_port,
 	.watchdog_ops = &mv88e6097_watchdog_ops,
@@ -5215,7 +5237,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -5277,7 +5299,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	.set_cpu_port = mv88e6390_g1_set_cpu_port,
 	.set_egress_port = mv88e6390_g1_set_egress_port,
 	.watchdog_ops = &mv88e6390_watchdog_ops,
@@ -5339,7 +5361,7 @@ static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
 	.stats_get_strings = mv88e6320_stats_get_strings,
-	.stats_get_stats = mv88e6390_stats_get_stats,
+	.stats_get_stat = mv88e6390_stats_get_stat,
 	/* .set_cpu_port is missing because this family does not support a global
 	 * CPU port, only per port CPU port which is set via
 	 * .port_set_upstream_port method.
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 44383a03ef2f..b0dfbcae0be9 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -318,6 +318,17 @@ struct mv88e6xxx_mst {
 	struct mv88e6xxx_stu_entry stu;
 };
 
+#define STATS_TYPE_PORT		BIT(0)
+#define STATS_TYPE_BANK0	BIT(1)
+#define STATS_TYPE_BANK1	BIT(2)
+
+struct mv88e6xxx_hw_stat {
+	char string[ETH_GSTRING_LEN];
+	size_t size;
+	int reg;
+	int type;
+};
+
 struct mv88e6xxx_chip {
 	const struct mv88e6xxx_info *info;
 
@@ -574,8 +585,9 @@ struct mv88e6xxx_ops {
 	/* Return the number of strings describing statistics */
 	int (*stats_get_sset_count)(struct mv88e6xxx_chip *chip);
 	int (*stats_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
-	int (*stats_get_stats)(struct mv88e6xxx_chip *chip,  int port,
-			       uint64_t *data);
+	int (*stats_get_stat)(struct mv88e6xxx_chip *chip,  int port,
+			      const struct mv88e6xxx_hw_stat *stat,
+			      uint64_t *data);
 	int (*set_cpu_port)(struct mv88e6xxx_chip *chip, int port);
 	int (*set_egress_port)(struct mv88e6xxx_chip *chip,
 			       enum mv88e6xxx_egress_direction direction,
@@ -727,17 +739,6 @@ struct mv88e6xxx_pcs_ops {
 
 };
 
-#define STATS_TYPE_PORT		BIT(0)
-#define STATS_TYPE_BANK0	BIT(1)
-#define STATS_TYPE_BANK1	BIT(2)
-
-struct mv88e6xxx_hw_stat {
-	char string[ETH_GSTRING_LEN];
-	size_t size;
-	int reg;
-	int type;
-};
-
 static inline bool mv88e6xxx_has_stu(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->max_sid > 0 &&
-- 
2.34.1

[PATCH net-next 2/4] net: dsa: mv88e6xxx: Give each hw stat an ID

[Tobias Waldekranz]

With the upcoming standard counter group support, we are no longer
reading out the whole set of counters, but rather mapping a subset to
the requested group.

Therefore, create an enum with an ID for each stat, such that
mv88e6xxx_hw_stats[] can be subscripted with a human-readable ID
corresponding to the counter's name.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 140 +++++++++++++++++--------------
 1 file changed, 77 insertions(+), 63 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ffd81174bda3..71c60f229a2f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -937,66 +937,80 @@ static int mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
 	return chip->info->ops->stats_snapshot(chip, port);
 }
 
-static struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
-	{ "in_good_octets",		8, 0x00, STATS_TYPE_BANK0, },
-	{ "in_bad_octets",		4, 0x02, STATS_TYPE_BANK0, },
-	{ "in_unicast",			4, 0x04, STATS_TYPE_BANK0, },
-	{ "in_broadcasts",		4, 0x06, STATS_TYPE_BANK0, },
-	{ "in_multicasts",		4, 0x07, STATS_TYPE_BANK0, },
-	{ "in_pause",			4, 0x16, STATS_TYPE_BANK0, },
-	{ "in_undersize",		4, 0x18, STATS_TYPE_BANK0, },
-	{ "in_fragments",		4, 0x19, STATS_TYPE_BANK0, },
-	{ "in_oversize",		4, 0x1a, STATS_TYPE_BANK0, },
-	{ "in_jabber",			4, 0x1b, STATS_TYPE_BANK0, },
-	{ "in_rx_error",		4, 0x1c, STATS_TYPE_BANK0, },
-	{ "in_fcs_error",		4, 0x1d, STATS_TYPE_BANK0, },
-	{ "out_octets",			8, 0x0e, STATS_TYPE_BANK0, },
-	{ "out_unicast",		4, 0x10, STATS_TYPE_BANK0, },
-	{ "out_broadcasts",		4, 0x13, STATS_TYPE_BANK0, },
-	{ "out_multicasts",		4, 0x12, STATS_TYPE_BANK0, },
-	{ "out_pause",			4, 0x15, STATS_TYPE_BANK0, },
-	{ "excessive",			4, 0x11, STATS_TYPE_BANK0, },
-	{ "collisions",			4, 0x1e, STATS_TYPE_BANK0, },
-	{ "deferred",			4, 0x05, STATS_TYPE_BANK0, },
-	{ "single",			4, 0x14, STATS_TYPE_BANK0, },
-	{ "multiple",			4, 0x17, STATS_TYPE_BANK0, },
-	{ "out_fcs_error",		4, 0x03, STATS_TYPE_BANK0, },
-	{ "late",			4, 0x1f, STATS_TYPE_BANK0, },
-	{ "hist_64bytes",		4, 0x08, STATS_TYPE_BANK0, },
-	{ "hist_65_127bytes",		4, 0x09, STATS_TYPE_BANK0, },
-	{ "hist_128_255bytes",		4, 0x0a, STATS_TYPE_BANK0, },
-	{ "hist_256_511bytes",		4, 0x0b, STATS_TYPE_BANK0, },
-	{ "hist_512_1023bytes",		4, 0x0c, STATS_TYPE_BANK0, },
-	{ "hist_1024_max_bytes",	4, 0x0d, STATS_TYPE_BANK0, },
-	{ "sw_in_discards",		4, 0x10, STATS_TYPE_PORT, },
-	{ "sw_in_filtered",		2, 0x12, STATS_TYPE_PORT, },
-	{ "sw_out_filtered",		2, 0x13, STATS_TYPE_PORT, },
-	{ "in_discards",		4, 0x00, STATS_TYPE_BANK1, },
-	{ "in_filtered",		4, 0x01, STATS_TYPE_BANK1, },
-	{ "in_accepted",		4, 0x02, STATS_TYPE_BANK1, },
-	{ "in_bad_accepted",		4, 0x03, STATS_TYPE_BANK1, },
-	{ "in_good_avb_class_a",	4, 0x04, STATS_TYPE_BANK1, },
-	{ "in_good_avb_class_b",	4, 0x05, STATS_TYPE_BANK1, },
-	{ "in_bad_avb_class_a",		4, 0x06, STATS_TYPE_BANK1, },
-	{ "in_bad_avb_class_b",		4, 0x07, STATS_TYPE_BANK1, },
-	{ "tcam_counter_0",		4, 0x08, STATS_TYPE_BANK1, },
-	{ "tcam_counter_1",		4, 0x09, STATS_TYPE_BANK1, },
-	{ "tcam_counter_2",		4, 0x0a, STATS_TYPE_BANK1, },
-	{ "tcam_counter_3",		4, 0x0b, STATS_TYPE_BANK1, },
-	{ "in_da_unknown",		4, 0x0e, STATS_TYPE_BANK1, },
-	{ "in_management",		4, 0x0f, STATS_TYPE_BANK1, },
-	{ "out_queue_0",		4, 0x10, STATS_TYPE_BANK1, },
-	{ "out_queue_1",		4, 0x11, STATS_TYPE_BANK1, },
-	{ "out_queue_2",		4, 0x12, STATS_TYPE_BANK1, },
-	{ "out_queue_3",		4, 0x13, STATS_TYPE_BANK1, },
-	{ "out_queue_4",		4, 0x14, STATS_TYPE_BANK1, },
-	{ "out_queue_5",		4, 0x15, STATS_TYPE_BANK1, },
-	{ "out_queue_6",		4, 0x16, STATS_TYPE_BANK1, },
-	{ "out_queue_7",		4, 0x17, STATS_TYPE_BANK1, },
-	{ "out_cut_through",		4, 0x18, STATS_TYPE_BANK1, },
-	{ "out_octets_a",		4, 0x1a, STATS_TYPE_BANK1, },
-	{ "out_octets_b",		4, 0x1b, STATS_TYPE_BANK1, },
-	{ "out_management",		4, 0x1f, STATS_TYPE_BANK1, },
+#define MV88E6XXX_HW_STAT_MAPPER(_fn)				   \
+	_fn(in_good_octets,		8, 0x00, STATS_TYPE_BANK0) \
+	_fn(in_bad_octets,		4, 0x02, STATS_TYPE_BANK0) \
+	_fn(in_unicast,			4, 0x04, STATS_TYPE_BANK0) \
+	_fn(in_broadcasts,		4, 0x06, STATS_TYPE_BANK0) \
+	_fn(in_multicasts,		4, 0x07, STATS_TYPE_BANK0) \
+	_fn(in_pause,			4, 0x16, STATS_TYPE_BANK0) \
+	_fn(in_undersize,		4, 0x18, STATS_TYPE_BANK0) \
+	_fn(in_fragments,		4, 0x19, STATS_TYPE_BANK0) \
+	_fn(in_oversize,		4, 0x1a, STATS_TYPE_BANK0) \
+	_fn(in_jabber,			4, 0x1b, STATS_TYPE_BANK0) \
+	_fn(in_rx_error,		4, 0x1c, STATS_TYPE_BANK0) \
+	_fn(in_fcs_error,		4, 0x1d, STATS_TYPE_BANK0) \
+	_fn(out_octets,			8, 0x0e, STATS_TYPE_BANK0) \
+	_fn(out_unicast,		4, 0x10, STATS_TYPE_BANK0) \
+	_fn(out_broadcasts,		4, 0x13, STATS_TYPE_BANK0) \
+	_fn(out_multicasts,		4, 0x12, STATS_TYPE_BANK0) \
+	_fn(out_pause,			4, 0x15, STATS_TYPE_BANK0) \
+	_fn(excessive,			4, 0x11, STATS_TYPE_BANK0) \
+	_fn(collisions,			4, 0x1e, STATS_TYPE_BANK0) \
+	_fn(deferred,			4, 0x05, STATS_TYPE_BANK0) \
+	_fn(single,			4, 0x14, STATS_TYPE_BANK0) \
+	_fn(multiple,			4, 0x17, STATS_TYPE_BANK0) \
+	_fn(out_fcs_error,		4, 0x03, STATS_TYPE_BANK0) \
+	_fn(late,			4, 0x1f, STATS_TYPE_BANK0) \
+	_fn(hist_64bytes,		4, 0x08, STATS_TYPE_BANK0) \
+	_fn(hist_65_127bytes,		4, 0x09, STATS_TYPE_BANK0) \
+	_fn(hist_128_255bytes,		4, 0x0a, STATS_TYPE_BANK0) \
+	_fn(hist_256_511bytes,		4, 0x0b, STATS_TYPE_BANK0) \
+	_fn(hist_512_1023bytes,		4, 0x0c, STATS_TYPE_BANK0) \
+	_fn(hist_1024_max_bytes,	4, 0x0d, STATS_TYPE_BANK0) \
+	_fn(sw_in_discards,		4, 0x10, STATS_TYPE_PORT)  \
+	_fn(sw_in_filtered,		2, 0x12, STATS_TYPE_PORT)  \
+	_fn(sw_out_filtered,		2, 0x13, STATS_TYPE_PORT)  \
+	_fn(in_discards,		4, 0x00, STATS_TYPE_BANK1) \
+	_fn(in_filtered,		4, 0x01, STATS_TYPE_BANK1) \
+	_fn(in_accepted,		4, 0x02, STATS_TYPE_BANK1) \
+	_fn(in_bad_accepted,		4, 0x03, STATS_TYPE_BANK1) \
+	_fn(in_good_avb_class_a,	4, 0x04, STATS_TYPE_BANK1) \
+	_fn(in_good_avb_class_b,	4, 0x05, STATS_TYPE_BANK1) \
+	_fn(in_bad_avb_class_a,		4, 0x06, STATS_TYPE_BANK1) \
+	_fn(in_bad_avb_class_b,		4, 0x07, STATS_TYPE_BANK1) \
+	_fn(tcam_counter_0,		4, 0x08, STATS_TYPE_BANK1) \
+	_fn(tcam_counter_1,		4, 0x09, STATS_TYPE_BANK1) \
+	_fn(tcam_counter_2,		4, 0x0a, STATS_TYPE_BANK1) \
+	_fn(tcam_counter_3,		4, 0x0b, STATS_TYPE_BANK1) \
+	_fn(in_da_unknown,		4, 0x0e, STATS_TYPE_BANK1) \
+	_fn(in_management,		4, 0x0f, STATS_TYPE_BANK1) \
+	_fn(out_queue_0,		4, 0x10, STATS_TYPE_BANK1) \
+	_fn(out_queue_1,		4, 0x11, STATS_TYPE_BANK1) \
+	_fn(out_queue_2,		4, 0x12, STATS_TYPE_BANK1) \
+	_fn(out_queue_3,		4, 0x13, STATS_TYPE_BANK1) \
+	_fn(out_queue_4,		4, 0x14, STATS_TYPE_BANK1) \
+	_fn(out_queue_5,		4, 0x15, STATS_TYPE_BANK1) \
+	_fn(out_queue_6,		4, 0x16, STATS_TYPE_BANK1) \
+	_fn(out_queue_7,		4, 0x17, STATS_TYPE_BANK1) \
+	_fn(out_cut_through,		4, 0x18, STATS_TYPE_BANK1) \
+	_fn(out_octets_a,		4, 0x1a, STATS_TYPE_BANK1) \
+	_fn(out_octets_b,		4, 0x1b, STATS_TYPE_BANK1) \
+	_fn(out_management,		4, 0x1f, STATS_TYPE_BANK1) \
+	/*  */
+
+#define MV88E6XXX_HW_STAT_ENTRY(_string, _size, _reg, _type) \
+	{ #_string, _size, _reg, _type },
+static const struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
+	MV88E6XXX_HW_STAT_MAPPER(MV88E6XXX_HW_STAT_ENTRY)
+};
+
+#define MV88E6XXX_HW_STAT_ENUM(_string, _size, _reg, _type) \
+	MV88E6XXX_HW_STAT_ID_ ## _string,
+enum mv88e6xxx_hw_stat_id {
+	MV88E6XXX_HW_STAT_MAPPER(MV88E6XXX_HW_STAT_ENUM)
+
+	MV88E6XXX_HW_STAT_ID_MAX
 };
 
 static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
@@ -1043,7 +1057,7 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
 static int mv88e6xxx_stats_get_strings(struct mv88e6xxx_chip *chip,
 				       uint8_t *data, int types)
 {
-	struct mv88e6xxx_hw_stat *stat;
+	const struct mv88e6xxx_hw_stat *stat;
 	int i, j;
 
 	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
@@ -1124,7 +1138,7 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
 static int mv88e6xxx_stats_get_sset_count(struct mv88e6xxx_chip *chip,
 					  int types)
 {
-	struct mv88e6xxx_hw_stat *stat;
+	const struct mv88e6xxx_hw_stat *stat;
 	int i, j;
 
 	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
@@ -1251,7 +1265,7 @@ static int mv88e6xxx_stats_get_stat(struct mv88e6xxx_chip *chip, int port,
 static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
 				     uint64_t *data)
 {
-	struct mv88e6xxx_hw_stat *stat;
+	const struct mv88e6xxx_hw_stat *stat;
 	int i, j;
 
 	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
-- 
2.34.1

[PATCH net-next 3/4] net: dsa: mv88e6xxx: Add "eth-mac" counter group support

[Tobias Waldekranz]

Report the applicable subset of an mv88e6xxx port's counters using
ethtool's standardized "eth-mac" counter group.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 52 ++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 71c60f229a2f..51e3744cb89b 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1320,6 +1320,57 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 
 }
 
+static void mv88e6xxx_get_eth_mac_stats(struct dsa_switch *ds, int port,
+					struct ethtool_eth_mac_stats *mac_stats)
+{
+#define MV88E6XXX_ETH_MAC_STAT_MAPPING(_id, _member)			\
+	[MV88E6XXX_HW_STAT_ID_ ## _id] =				\
+		offsetof(struct ethtool_eth_mac_stats, stats._member)	\
+
+	static const size_t stat_map[MV88E6XXX_HW_STAT_ID_MAX] = {
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_unicast, FramesTransmittedOK),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(single, SingleCollisionFrames),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(multiple, MultipleCollisionFrames),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_unicast, FramesReceivedOK),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_fcs_error, FrameCheckSequenceErrors),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_octets, OctetsTransmittedOK),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(deferred, FramesWithDeferredXmissions),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(late, LateCollisions),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_good_octets, OctetsReceivedOK),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_multicasts, MulticastFramesXmittedOK),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_broadcasts, BroadcastFramesXmittedOK),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(excessive, FramesWithExcessiveDeferral),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_multicasts, MulticastFramesReceivedOK),
+		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_broadcasts, BroadcastFramesReceivedOK),
+	};
+	struct mv88e6xxx_chip *chip = ds->priv;
+	const struct mv88e6xxx_hw_stat *stat;
+	enum mv88e6xxx_hw_stat_id id;
+	u64 *member;
+	int ret;
+
+	mv88e6xxx_reg_lock(chip);
+	ret = mv88e6xxx_stats_snapshot(chip, port);
+	mv88e6xxx_reg_unlock(chip);
+
+	if (ret < 0)
+		return;
+
+	stat = mv88e6xxx_hw_stats;
+	for (id = 0; id < MV88E6XXX_HW_STAT_ID_MAX; id++, stat++) {
+		if (!stat_map[id])
+			continue;
+
+		member = (u64 *)(((char *)mac_stats) + stat_map[id]);
+		mv88e6xxx_stats_get_stat(chip, port, stat, member);
+	}
+
+	mac_stats->stats.FramesTransmittedOK += mac_stats->stats.MulticastFramesXmittedOK;
+	mac_stats->stats.FramesTransmittedOK += mac_stats->stats.BroadcastFramesXmittedOK;
+	mac_stats->stats.FramesReceivedOK += mac_stats->stats.MulticastFramesReceivedOK;
+	mac_stats->stats.FramesReceivedOK += mac_stats->stats.BroadcastFramesReceivedOK;
+}
+
 static int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
@@ -6839,6 +6890,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.phylink_mac_link_up	= mv88e6xxx_mac_link_up,
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
+	.get_eth_mac_stats	= mv88e6xxx_get_eth_mac_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
 	.port_max_mtu		= mv88e6xxx_get_max_mtu,
 	.port_change_mtu	= mv88e6xxx_change_mtu,
-- 
2.34.1

[PATCH net-next 4/4] net: dsa: mv88e6xxx: Add "rmon" counter group support

[Tobias Waldekranz]

Report the applicable subset of an mv88e6xxx port's counters using
ethtool's standardized "rmon" counter group.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 55 ++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 51e3744cb89b..bcb47ccf3ec1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1371,6 +1371,60 @@ static void mv88e6xxx_get_eth_mac_stats(struct dsa_switch *ds, int port,
 	mac_stats->stats.FramesReceivedOK += mac_stats->stats.BroadcastFramesReceivedOK;
 }
 
+static void mv88e6xxx_get_rmon_stats(struct dsa_switch *ds, int port,
+				     struct ethtool_rmon_stats *rmon_stats,
+				     const struct ethtool_rmon_hist_range **ranges)
+{
+#define MV88E6XXX_RMON_STAT_MAPPING(_id, _member)			\
+	[MV88E6XXX_HW_STAT_ID_ ## _id] =				\
+		offsetof(struct ethtool_rmon_stats, stats._member)	\
+
+	static const size_t stat_map[MV88E6XXX_HW_STAT_ID_MAX] = {
+		MV88E6XXX_RMON_STAT_MAPPING(in_undersize, undersize_pkts),
+		MV88E6XXX_RMON_STAT_MAPPING(in_oversize, oversize_pkts),
+		MV88E6XXX_RMON_STAT_MAPPING(in_fragments, fragments),
+		MV88E6XXX_RMON_STAT_MAPPING(in_jabber, jabbers),
+		MV88E6XXX_RMON_STAT_MAPPING(hist_64bytes, hist[0]),
+		MV88E6XXX_RMON_STAT_MAPPING(hist_65_127bytes, hist[1]),
+		MV88E6XXX_RMON_STAT_MAPPING(hist_128_255bytes, hist[2]),
+		MV88E6XXX_RMON_STAT_MAPPING(hist_256_511bytes, hist[3]),
+		MV88E6XXX_RMON_STAT_MAPPING(hist_512_1023bytes, hist[4]),
+		MV88E6XXX_RMON_STAT_MAPPING(hist_1024_max_bytes, hist[5]),
+	};
+	static const struct ethtool_rmon_hist_range rmon_ranges[] = {
+		{   64,    64 },
+		{   65,   127 },
+		{  128,   255 },
+		{  256,   511 },
+		{  512,  1023 },
+		{ 1024, 65535 },
+		{}
+	};
+	struct mv88e6xxx_chip *chip = ds->priv;
+	const struct mv88e6xxx_hw_stat *stat;
+	enum mv88e6xxx_hw_stat_id id;
+	u64 *member;
+	int ret;
+
+	mv88e6xxx_reg_lock(chip);
+	ret = mv88e6xxx_stats_snapshot(chip, port);
+	mv88e6xxx_reg_unlock(chip);
+
+	if (ret < 0)
+		return;
+
+	stat = mv88e6xxx_hw_stats;
+	for (id = 0; id < MV88E6XXX_HW_STAT_ID_MAX; id++, stat++) {
+		if (!stat_map[id])
+			continue;
+
+		member = (u64 *)(((char *)rmon_stats) + stat_map[id]);
+		mv88e6xxx_stats_get_stat(chip, port, stat, member);
+	}
+
+	*ranges = rmon_ranges;
+}
+
 static int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
@@ -6891,6 +6945,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.get_strings		= mv88e6xxx_get_strings,
 	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
 	.get_eth_mac_stats	= mv88e6xxx_get_eth_mac_stats,
+	.get_rmon_stats		= mv88e6xxx_get_rmon_stats,
 	.get_sset_count		= mv88e6xxx_get_sset_count,
 	.port_max_mtu		= mv88e6xxx_get_max_mtu,
 	.port_change_mtu	= mv88e6xxx_change_mtu,
-- 
2.34.1

Re: [PATCH net-next 1/4] net: dsa: mv88e6xxx: Create API to read a single stat counter

[Vladimir Oltean]

On Fri, Dec 01, 2023 at 01:58:09PM +0100, Tobias Waldekranz wrote:
> This change contains no functional change. We simply push the hardware
> specific stats logic to a function reading a single counter, rather
> than the whole set.
> 
> This is a preparatory change for the upcoming standard ethtool
> statistics support (i.e. "eth-mac", "eth-ctrl" etc.).
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---

I think I like this patch set. Some nitpicks below.

> -static int mv88e6320_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
> -				     uint64_t *data)
> +static int mv88e6xxx_stats_get_stat(struct mv88e6xxx_chip *chip, int port,
> +				    const struct mv88e6xxx_hw_stat *stat,
> +				    uint64_t *data)
>  {
> -	return mv88e6xxx_stats_get_stats(chip, port, data,
> -					 STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
> -					 MV88E6XXX_G1_STATS_OP_BANK_1_BIT_9,
> -					 MV88E6XXX_G1_STATS_OP_HIST_RX_TX);
> +	int ret = 0;
> +
> +	if (chip->info->ops->stats_get_stat) {
> +		mv88e6xxx_reg_lock(chip);
> +		ret = chip->info->ops->stats_get_stat(chip, port, stat, data);
> +		mv88e6xxx_reg_unlock(chip);
> +	}

There is no chip->info->ops which does not provide stats_get_stat().
As a separate change, I suppose you could drop the "if".

> +
> +	return ret;
>  }
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
> index 44383a03ef2f..b0dfbcae0be9 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.h
> +++ b/drivers/net/dsa/mv88e6xxx/chip.h
> @@ -574,8 +585,9 @@ struct mv88e6xxx_ops {
>  	/* Return the number of strings describing statistics */
>  	int (*stats_get_sset_count)(struct mv88e6xxx_chip *chip);
>  	int (*stats_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
> -	int (*stats_get_stats)(struct mv88e6xxx_chip *chip,  int port,
> -			       uint64_t *data);
> +	int (*stats_get_stat)(struct mv88e6xxx_chip *chip,  int port,
> +			      const struct mv88e6xxx_hw_stat *stat,
> +			      uint64_t *data);

Since you are touching this function prototype anyway, I guess you could
change its return type. Int assumes it can also return a negative error
code, which it doesn't. Maybe size_t to denote that it returns an unsigned
count of statistics?

Also, there is an extraneous space before the "int port" argument which
you could fix up now.

>  	int (*set_cpu_port)(struct mv88e6xxx_chip *chip, int port);
>  	int (*set_egress_port)(struct mv88e6xxx_chip *chip,
>  			       enum mv88e6xxx_egress_direction direction,

Re: [PATCH net-next 2/4] net: dsa: mv88e6xxx: Give each hw stat an ID

[Vladimir Oltean]

On Fri, Dec 01, 2023 at 01:58:10PM +0100, Tobias Waldekranz wrote:
> +#define MV88E6XXX_HW_STAT_MAPPER(_fn)				   \
> +	_fn(in_good_octets,		8, 0x00, STATS_TYPE_BANK0) \
> +	_fn(in_bad_octets,		4, 0x02, STATS_TYPE_BANK0) \
> +	_fn(in_unicast,			4, 0x04, STATS_TYPE_BANK0) \
> +	_fn(in_broadcasts,		4, 0x06, STATS_TYPE_BANK0) \
> +	_fn(in_multicasts,		4, 0x07, STATS_TYPE_BANK0) \
> +	_fn(in_pause,			4, 0x16, STATS_TYPE_BANK0) \
> +	_fn(in_undersize,		4, 0x18, STATS_TYPE_BANK0) \
> +	_fn(in_fragments,		4, 0x19, STATS_TYPE_BANK0) \
> +	_fn(in_oversize,		4, 0x1a, STATS_TYPE_BANK0) \
> +	_fn(in_jabber,			4, 0x1b, STATS_TYPE_BANK0) \
> +	_fn(in_rx_error,		4, 0x1c, STATS_TYPE_BANK0) \
> +	_fn(in_fcs_error,		4, 0x1d, STATS_TYPE_BANK0) \
> +	_fn(out_octets,			8, 0x0e, STATS_TYPE_BANK0) \
> +	_fn(out_unicast,		4, 0x10, STATS_TYPE_BANK0) \
> +	_fn(out_broadcasts,		4, 0x13, STATS_TYPE_BANK0) \
> +	_fn(out_multicasts,		4, 0x12, STATS_TYPE_BANK0) \
> +	_fn(out_pause,			4, 0x15, STATS_TYPE_BANK0) \
> +	_fn(excessive,			4, 0x11, STATS_TYPE_BANK0) \
> +	_fn(collisions,			4, 0x1e, STATS_TYPE_BANK0) \
> +	_fn(deferred,			4, 0x05, STATS_TYPE_BANK0) \
> +	_fn(single,			4, 0x14, STATS_TYPE_BANK0) \
> +	_fn(multiple,			4, 0x17, STATS_TYPE_BANK0) \
> +	_fn(out_fcs_error,		4, 0x03, STATS_TYPE_BANK0) \
> +	_fn(late,			4, 0x1f, STATS_TYPE_BANK0) \
> +	_fn(hist_64bytes,		4, 0x08, STATS_TYPE_BANK0) \
> +	_fn(hist_65_127bytes,		4, 0x09, STATS_TYPE_BANK0) \
> +	_fn(hist_128_255bytes,		4, 0x0a, STATS_TYPE_BANK0) \
> +	_fn(hist_256_511bytes,		4, 0x0b, STATS_TYPE_BANK0) \
> +	_fn(hist_512_1023bytes,		4, 0x0c, STATS_TYPE_BANK0) \
> +	_fn(hist_1024_max_bytes,	4, 0x0d, STATS_TYPE_BANK0) \
> +	_fn(sw_in_discards,		4, 0x10, STATS_TYPE_PORT)  \
> +	_fn(sw_in_filtered,		2, 0x12, STATS_TYPE_PORT)  \
> +	_fn(sw_out_filtered,		2, 0x13, STATS_TYPE_PORT)  \
> +	_fn(in_discards,		4, 0x00, STATS_TYPE_BANK1) \
> +	_fn(in_filtered,		4, 0x01, STATS_TYPE_BANK1) \
> +	_fn(in_accepted,		4, 0x02, STATS_TYPE_BANK1) \
> +	_fn(in_bad_accepted,		4, 0x03, STATS_TYPE_BANK1) \
> +	_fn(in_good_avb_class_a,	4, 0x04, STATS_TYPE_BANK1) \
> +	_fn(in_good_avb_class_b,	4, 0x05, STATS_TYPE_BANK1) \
> +	_fn(in_bad_avb_class_a,		4, 0x06, STATS_TYPE_BANK1) \
> +	_fn(in_bad_avb_class_b,		4, 0x07, STATS_TYPE_BANK1) \
> +	_fn(tcam_counter_0,		4, 0x08, STATS_TYPE_BANK1) \
> +	_fn(tcam_counter_1,		4, 0x09, STATS_TYPE_BANK1) \
> +	_fn(tcam_counter_2,		4, 0x0a, STATS_TYPE_BANK1) \
> +	_fn(tcam_counter_3,		4, 0x0b, STATS_TYPE_BANK1) \
> +	_fn(in_da_unknown,		4, 0x0e, STATS_TYPE_BANK1) \
> +	_fn(in_management,		4, 0x0f, STATS_TYPE_BANK1) \
> +	_fn(out_queue_0,		4, 0x10, STATS_TYPE_BANK1) \
> +	_fn(out_queue_1,		4, 0x11, STATS_TYPE_BANK1) \
> +	_fn(out_queue_2,		4, 0x12, STATS_TYPE_BANK1) \
> +	_fn(out_queue_3,		4, 0x13, STATS_TYPE_BANK1) \
> +	_fn(out_queue_4,		4, 0x14, STATS_TYPE_BANK1) \
> +	_fn(out_queue_5,		4, 0x15, STATS_TYPE_BANK1) \
> +	_fn(out_queue_6,		4, 0x16, STATS_TYPE_BANK1) \
> +	_fn(out_queue_7,		4, 0x17, STATS_TYPE_BANK1) \
> +	_fn(out_cut_through,		4, 0x18, STATS_TYPE_BANK1) \
> +	_fn(out_octets_a,		4, 0x1a, STATS_TYPE_BANK1) \
> +	_fn(out_octets_b,		4, 0x1b, STATS_TYPE_BANK1) \
> +	_fn(out_management,		4, 0x1f, STATS_TYPE_BANK1) \
> +	/*  */
> +
> +#define MV88E6XXX_HW_STAT_ENTRY(_string, _size, _reg, _type) \
> +	{ #_string, _size, _reg, _type },

I think it would look better to take the comma out of this macro and put
it into its callers, so that enums and arrays have the natural separators
between elements.

> +static const struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
> +	MV88E6XXX_HW_STAT_MAPPER(MV88E6XXX_HW_STAT_ENTRY)
> +};
> +
> +#define MV88E6XXX_HW_STAT_ENUM(_string, _size, _reg, _type) \
> +	MV88E6XXX_HW_STAT_ID_ ## _string,
> +enum mv88e6xxx_hw_stat_id {
> +	MV88E6XXX_HW_STAT_MAPPER(MV88E6XXX_HW_STAT_ENUM)
> +
> +	MV88E6XXX_HW_STAT_ID_MAX
>  };

Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: Add "eth-mac" counter group support

[Vladimir Oltean]

On Fri, Dec 01, 2023 at 01:58:11PM +0100, Tobias Waldekranz wrote:
> Report the applicable subset of an mv88e6xxx port's counters using
> ethtool's standardized "eth-mac" counter group.
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 52 ++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 71c60f229a2f..51e3744cb89b 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1320,6 +1320,57 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
>  
>  }
>  
> +static void mv88e6xxx_get_eth_mac_stats(struct dsa_switch *ds, int port,
> +					struct ethtool_eth_mac_stats *mac_stats)
> +{
> +#define MV88E6XXX_ETH_MAC_STAT_MAPPING(_id, _member)			\
> +	[MV88E6XXX_HW_STAT_ID_ ## _id] =				\
> +		offsetof(struct ethtool_eth_mac_stats, stats._member)	\
> +
> +	static const size_t stat_map[MV88E6XXX_HW_STAT_ID_MAX] = {
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_unicast, FramesTransmittedOK),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(single, SingleCollisionFrames),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(multiple, MultipleCollisionFrames),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_unicast, FramesReceivedOK),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_fcs_error, FrameCheckSequenceErrors),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_octets, OctetsTransmittedOK),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(deferred, FramesWithDeferredXmissions),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(late, LateCollisions),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_good_octets, OctetsReceivedOK),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_multicasts, MulticastFramesXmittedOK),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(out_broadcasts, BroadcastFramesXmittedOK),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(excessive, FramesWithExcessiveDeferral),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_multicasts, MulticastFramesReceivedOK),
> +		MV88E6XXX_ETH_MAC_STAT_MAPPING(in_broadcasts, BroadcastFramesReceivedOK),
> +	};
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	const struct mv88e6xxx_hw_stat *stat;
> +	enum mv88e6xxx_hw_stat_id id;
> +	u64 *member;
> +	int ret;
> +
> +	mv88e6xxx_reg_lock(chip);
> +	ret = mv88e6xxx_stats_snapshot(chip, port);
> +	mv88e6xxx_reg_unlock(chip);

Wouldn't it be more consistent with this driver's own convention if the
mv88e6xxx_reg_lock() was absorbed by mv88e6xxx_stats_snapshot()?

> +
> +	if (ret < 0)
> +		return;
> +
> +	stat = mv88e6xxx_hw_stats;
> +	for (id = 0; id < MV88E6XXX_HW_STAT_ID_MAX; id++, stat++) {
> +		if (!stat_map[id])
> +			continue;

Hmm, a bit fragile, this relies on offsetof(struct ethtool_eth_mac_stats, stats)
never being 0, which is not a documented requirement.

Would it be hard to make the stat_map[] array compressed rather than
sparse, and push the stat id inside the struct mv88e6xxx_hw_stat, and
only operate on ARRAY_SIZE() elements?

> +
> +		member = (u64 *)(((char *)mac_stats) + stat_map[id]);
> +		mv88e6xxx_stats_get_stat(chip, port, stat, member);
> +	}
> +
> +	mac_stats->stats.FramesTransmittedOK += mac_stats->stats.MulticastFramesXmittedOK;
> +	mac_stats->stats.FramesTransmittedOK += mac_stats->stats.BroadcastFramesXmittedOK;
> +	mac_stats->stats.FramesReceivedOK += mac_stats->stats.MulticastFramesReceivedOK;
> +	mac_stats->stats.FramesReceivedOK += mac_stats->stats.BroadcastFramesReceivedOK;
> +}
> +
>  static int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
>  {
>  	struct mv88e6xxx_chip *chip = ds->priv;
> @@ -6839,6 +6890,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
>  	.phylink_mac_link_up	= mv88e6xxx_mac_link_up,
>  	.get_strings		= mv88e6xxx_get_strings,
>  	.get_ethtool_stats	= mv88e6xxx_get_ethtool_stats,
> +	.get_eth_mac_stats	= mv88e6xxx_get_eth_mac_stats,
>  	.get_sset_count		= mv88e6xxx_get_sset_count,
>  	.port_max_mtu		= mv88e6xxx_get_max_mtu,
>  	.port_change_mtu	= mv88e6xxx_change_mtu,
> -- 
> 2.34.1
> 

Re: [PATCH net-next 0/4] net: dsa: mv88e6xxx: Add "eth-mac" and "rmon" counter group support

[Vladimir Oltean]

On Fri, Dec 01, 2023 at 01:58:08PM +0100, Tobias Waldekranz wrote:
> 2/4 tries to collect all information about a stat in a single place
> using a mapper macro, which is then used to generate the original list
> of stats, along with a matching enum. checkpatch is less than amused
> with this construct, but prior art exists (__BPF_FUNC_MAPPER in
> include/uapi/linux/bpf.h, for example). Is there a better way forward?

Take checkpatch with salt?