diff mbox series

[net-next,v12,5/6] net: dsa: mv88e6xxx: rmon: Use RMU for reading RMON data

Message ID 20220915143658.3377139-6-mattias.forsblad@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: qca8k, mv88e6xxx: rmon: Add RMU support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning CHECK: Prefer kernel type 'u64' over 'uint64_t'
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Mattias Forsblad Sept. 15, 2022, 2:36 p.m. UTC 
Use the Remote Management Unit for efficiently accessing
the RMON data.

Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 180 ++++++++++++++++++++-----------
 drivers/net/dsa/mv88e6xxx/chip.h |   3 +
 drivers/net/dsa/mv88e6xxx/smi.c  |   3 +
 3 files changed, 121 insertions(+), 65 deletions(-)

Comments

Florian Fainelli Sept. 15, 2022, 8:11 p.m. UTC | #1 
On 9/15/22 07:36, Mattias Forsblad wrote:
> Use the Remote Management Unit for efficiently accessing
> the RMON data.
> 
> Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>

Seems a bit wasteful to add a rmu_reg field for each statistics member 
when there are only 6 STATS_TYPE_PORT that could be read over RMU.

Maybe what you could do is that the offset in the "reg" member could be 
split into a lower half that is used to encore the existing offset, and 
you use the upper half to encode the RMU-offset.
Andrew Lunn Sept. 15, 2022, 8:39 p.m. UTC | #2 
> +	{ "sw_in_discards",		4, 0x10, 0x81, STATS_TYPE_PORT, },
> +	{ "sw_in_filtered",		2, 0x12, 0x85, STATS_TYPE_PORT, },
> +	{ "sw_out_filtered",		2, 0x13, 0x89, STATS_TYPE_PORT, },

I agree with Florian here. Maybe add a table just for these three
values? Or even a switch statement.

> -static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
> -				     uint64_t *data, int types,
> -				     u16 bank1_select, u16 histogram)
> +static int mv88e6xxx_state_get_stats_rmu(struct mv88e6xxx_chip *chip, int port,
> +					 uint64_t *data, int types,
> +					 u16 bank1_select, u16 histogram)
> +{
> +	const u64 *stats = chip->ports[port].rmu_raw_stats;
> +	struct mv88e6xxx_hw_stat *stat;
> +	int offset = 0;
> +	u64 high;
> +	int i, j;
> +
> +	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
> +		stat = &mv88e6xxx_hw_stats[i];
> +		if (stat->type & types) {
> +			if (stat->type & STATS_TYPE_PORT) {
> +				data[j] = stats[stat->rmu_reg];
> +			} else {
> +				if (stat->type & STATS_TYPE_BANK1)
> +					offset = 32;
> +
> +				data[j] = stats[stat->reg + offset];
> +				if (stat->size == 8) {
> +					high = stats[stat->reg + offset + 1];
> +					data[j] += (high << 32);
> +				}
> +			}
> +
> +			j++;
> +		}
> +	}
> +	return j;
> +}

This is definitely getting better, closer to what i was thinking. But...

> +static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
> +				     uint64_t *data, int types,
> +				     u16 bank1_select, u16 histogram)
> +{
> +	if (mv88e6xxx_rmu_available(chip))
> +		return mv88e6xxx_state_get_stats_rmu(chip, port, data, types,
> +						     bank1_select, histogram);
> +	else
> +		return mv88e6xxx_stats_get_stats_mdio(chip, port, data, types,
> +						      bank1_select, histogram);
> +}

I would still like to get rid of this.

What i think the problem here is, you are trying to be 100%
compatible. So you only show bank1 statistics, if they where available
via MDIO. However, it seems like all the RMU implementations make all
statistics available, both banks, and counters in the port space.

So long as you keep the names consistent, you can return more
statistics via RMU than via MDIO.

   Andrew
Mattias Forsblad Sept. 16, 2022, 6:37 a.m. UTC | #3 
On 2022-09-15 22:11, Florian Fainelli wrote:
> On 9/15/22 07:36, Mattias Forsblad wrote:
>> Use the Remote Management Unit for efficiently accessing
>> the RMON data.
>>
>> Signed-off-by: Mattias Forsblad <mattias.forsblad@gmail.com>
> 
> Seems a bit wasteful to add a rmu_reg field for each statistics member when there are only 6 STATS_TYPE_PORT that could be read over RMU.
> 
> Maybe what you could do is that the offset in the "reg" member could be split into a lower half that is used to encore the existing offset, and you use the upper half to encode the RMU-offset.

As it's only three port statistics counter, I'll make a switch statement per Andrew suggestion. Thanks.

/Mattias
diff mbox series

Patch

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 294bf9bbaf3f..cba4f6a49647 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -984,65 +984,65 @@  static int mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int 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, },
+	{ "in_good_octets",		8, 0x00, 0x00, STATS_TYPE_BANK0, },
+	{ "in_bad_octets",		4, 0x02, 0x00, STATS_TYPE_BANK0, },
+	{ "in_unicast",			4, 0x04, 0x00, STATS_TYPE_BANK0, },
+	{ "in_broadcasts",		4, 0x06, 0x00, STATS_TYPE_BANK0, },
+	{ "in_multicasts",		4, 0x07, 0x00, STATS_TYPE_BANK0, },
+	{ "in_pause",			4, 0x16, 0x00, STATS_TYPE_BANK0, },
+	{ "in_undersize",		4, 0x18, 0x00, STATS_TYPE_BANK0, },
+	{ "in_fragments",		4, 0x19, 0x00, STATS_TYPE_BANK0, },
+	{ "in_oversize",		4, 0x1a, 0x00, STATS_TYPE_BANK0, },
+	{ "in_jabber",			4, 0x1b, 0x00, STATS_TYPE_BANK0, },
+	{ "in_rx_error",		4, 0x1c, 0x00, STATS_TYPE_BANK0, },
+	{ "in_fcs_error",		4, 0x1d, 0x00, STATS_TYPE_BANK0, },
+	{ "out_octets",			8, 0x0e, 0x00, STATS_TYPE_BANK0, },
+	{ "out_unicast",		4, 0x10, 0x00, STATS_TYPE_BANK0, },
+	{ "out_broadcasts",		4, 0x13, 0x00, STATS_TYPE_BANK0, },
+	{ "out_multicasts",		4, 0x12, 0x00, STATS_TYPE_BANK0, },
+	{ "out_pause",			4, 0x15, 0x00, STATS_TYPE_BANK0, },
+	{ "excessive",			4, 0x11, 0x00, STATS_TYPE_BANK0, },
+	{ "collisions",			4, 0x1e, 0x00, STATS_TYPE_BANK0, },
+	{ "deferred",			4, 0x05, 0x00, STATS_TYPE_BANK0, },
+	{ "single",			4, 0x14, 0x00, STATS_TYPE_BANK0, },
+	{ "multiple",			4, 0x17, 0x00, STATS_TYPE_BANK0, },
+	{ "out_fcs_error",		4, 0x03, 0x00, STATS_TYPE_BANK0, },
+	{ "late",			4, 0x1f, 0x00, STATS_TYPE_BANK0, },
+	{ "hist_64bytes",		4, 0x08, 0x00, STATS_TYPE_BANK0, },
+	{ "hist_65_127bytes",		4, 0x09, 0x00, STATS_TYPE_BANK0, },
+	{ "hist_128_255bytes",		4, 0x0a, 0x00, STATS_TYPE_BANK0, },
+	{ "hist_256_511bytes",		4, 0x0b, 0x00, STATS_TYPE_BANK0, },
+	{ "hist_512_1023bytes",		4, 0x0c, 0x00, STATS_TYPE_BANK0, },
+	{ "hist_1024_max_bytes",	4, 0x0d, 0x00, STATS_TYPE_BANK0, },
+	{ "sw_in_discards",		4, 0x10, 0x81, STATS_TYPE_PORT, },
+	{ "sw_in_filtered",		2, 0x12, 0x85, STATS_TYPE_PORT, },
+	{ "sw_out_filtered",		2, 0x13, 0x89, STATS_TYPE_PORT, },
+	{ "in_discards",		4, 0x00, 0x00, STATS_TYPE_BANK1, },
+	{ "in_filtered",		4, 0x01, 0x00, STATS_TYPE_BANK1, },
+	{ "in_accepted",		4, 0x02, 0x00, STATS_TYPE_BANK1, },
+	{ "in_bad_accepted",		4, 0x03, 0x00, STATS_TYPE_BANK1, },
+	{ "in_good_avb_class_a",	4, 0x04, 0x00, STATS_TYPE_BANK1, },
+	{ "in_good_avb_class_b",	4, 0x05, 0x00, STATS_TYPE_BANK1, },
+	{ "in_bad_avb_class_a",		4, 0x06, 0x00, STATS_TYPE_BANK1, },
+	{ "in_bad_avb_class_b",		4, 0x07, 0x00, STATS_TYPE_BANK1, },
+	{ "tcam_counter_0",		4, 0x08, 0x00, STATS_TYPE_BANK1, },
+	{ "tcam_counter_1",		4, 0x09, 0x00, STATS_TYPE_BANK1, },
+	{ "tcam_counter_2",		4, 0x0a, 0x00, STATS_TYPE_BANK1, },
+	{ "tcam_counter_3",		4, 0x0b, 0x00, STATS_TYPE_BANK1, },
+	{ "in_da_unknown",		4, 0x0e, 0x00, STATS_TYPE_BANK1, },
+	{ "in_management",		4, 0x0f, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_0",		4, 0x10, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_1",		4, 0x11, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_2",		4, 0x12, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_3",		4, 0x13, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_4",		4, 0x14, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_5",		4, 0x15, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_6",		4, 0x16, 0x00, STATS_TYPE_BANK1, },
+	{ "out_queue_7",		4, 0x17, 0x00, STATS_TYPE_BANK1, },
+	{ "out_cut_through",		4, 0x18, 0x00, STATS_TYPE_BANK1, },
+	{ "out_octets_a",		4, 0x1a, 0x00, STATS_TYPE_BANK1, },
+	{ "out_octets_b",		4, 0x1b, 0x00, STATS_TYPE_BANK1, },
+	{ "out_management",		4, 0x1f, 0x00, STATS_TYPE_BANK1, },
 };
 
 static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
@@ -1229,9 +1229,41 @@  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 mv88e6xxx_state_get_stats_rmu(struct mv88e6xxx_chip *chip, int port,
+					 uint64_t *data, int types,
+					 u16 bank1_select, u16 histogram)
+{
+	const u64 *stats = chip->ports[port].rmu_raw_stats;
+	struct mv88e6xxx_hw_stat *stat;
+	int offset = 0;
+	u64 high;
+	int i, j;
+
+	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
+		stat = &mv88e6xxx_hw_stats[i];
+		if (stat->type & types) {
+			if (stat->type & STATS_TYPE_PORT) {
+				data[j] = stats[stat->rmu_reg];
+			} else {
+				if (stat->type & STATS_TYPE_BANK1)
+					offset = 32;
+
+				data[j] = stats[stat->reg + offset];
+				if (stat->size == 8) {
+					high = stats[stat->reg + offset + 1];
+					data[j] += (high << 32);
+				}
+			}
+
+			j++;
+		}
+	}
+	return j;
+}
+
+static int mv88e6xxx_stats_get_stats_mdio(struct mv88e6xxx_chip *chip, int port,
+					  uint64_t *data, int types,
+					  u16 bank1_select, u16 histogram)
 {
 	struct mv88e6xxx_hw_stat *stat;
 	int i, j;
@@ -1251,6 +1283,18 @@  static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
 	return j;
 }
 
+static int mv88e6xxx_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
+				     uint64_t *data, int types,
+				     u16 bank1_select, u16 histogram)
+{
+	if (mv88e6xxx_rmu_available(chip))
+		return mv88e6xxx_state_get_stats_rmu(chip, port, data, types,
+						     bank1_select, histogram);
+	else
+		return mv88e6xxx_stats_get_stats_mdio(chip, port, data, types,
+						      bank1_select, histogram);
+}
+
 static int mv88e6095_stats_get_stats(struct mv88e6xxx_chip *chip, int port,
 				     uint64_t *data)
 {
@@ -1312,10 +1356,9 @@  static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
 	mv88e6xxx_reg_unlock(chip);
 }
 
-static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
-					uint64_t *data)
+void mv88e6xxx_get_ethtool_stats_mdio(struct mv88e6xxx_chip *chip, int port,
+				      uint64_t *data)
 {
-	struct mv88e6xxx_chip *chip = ds->priv;
 	int ret;
 
 	mv88e6xxx_reg_lock(chip);
@@ -1327,7 +1370,14 @@  static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 		return;
 
 	mv88e6xxx_get_stats(chip, port, data);
+}
+
+static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
+					uint64_t *data)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
 
+	chip->smi_ops->get_rmon(chip, port, data);
 }
 
 static int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index ea1789feeacf..ea86532f0bd9 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -742,6 +742,7 @@  struct mv88e6xxx_hw_stat {
 	char string[ETH_GSTRING_LEN];
 	size_t size;
 	int reg;
+	int rmu_reg;
 	int type;
 };
 
@@ -809,6 +810,8 @@  int mv88e6xxx_wait_mask(struct mv88e6xxx_chip *chip, int addr, int reg,
 int mv88e6xxx_wait_bit(struct mv88e6xxx_chip *chip, int addr, int reg,
 		       int bit, int val);
 struct mii_bus *mv88e6xxx_default_mdio_bus(struct mv88e6xxx_chip *chip);
+void mv88e6xxx_get_ethtool_stats_mdio(struct mv88e6xxx_chip *chip, int port,
+				      uint64_t *data);
 
 static inline void mv88e6xxx_reg_lock(struct mv88e6xxx_chip *chip)
 {
diff --git a/drivers/net/dsa/mv88e6xxx/smi.c b/drivers/net/dsa/mv88e6xxx/smi.c
index a990271b7482..ae805c449b85 100644
--- a/drivers/net/dsa/mv88e6xxx/smi.c
+++ b/drivers/net/dsa/mv88e6xxx/smi.c
@@ -83,6 +83,7 @@  static int mv88e6xxx_smi_direct_wait(struct mv88e6xxx_chip *chip,
 static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_direct_ops = {
 	.read = mv88e6xxx_smi_direct_read,
 	.write = mv88e6xxx_smi_direct_write,
+	.get_rmon = mv88e6xxx_get_ethtool_stats_mdio,
 };
 
 static int mv88e6xxx_smi_dual_direct_read(struct mv88e6xxx_chip *chip,
@@ -100,6 +101,7 @@  static int mv88e6xxx_smi_dual_direct_write(struct mv88e6xxx_chip *chip,
 static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_dual_direct_ops = {
 	.read = mv88e6xxx_smi_dual_direct_read,
 	.write = mv88e6xxx_smi_dual_direct_write,
+	.get_rmon = mv88e6xxx_get_ethtool_stats_mdio,
 };
 
 /* Offset 0x00: SMI Command Register
@@ -166,6 +168,7 @@  static const struct mv88e6xxx_bus_ops mv88e6xxx_smi_indirect_ops = {
 	.read = mv88e6xxx_smi_indirect_read,
 	.write = mv88e6xxx_smi_indirect_write,
 	.init = mv88e6xxx_smi_indirect_init,
+	.get_rmon = mv88e6xxx_get_ethtool_stats_mdio,
 };
 
 int mv88e6xxx_smi_init(struct mv88e6xxx_chip *chip,