[PATCH net-next 00/11] Start adding support for mv88e6390 family

[PATCH net-next 00/11] Start adding support for mv88e6390 family

[Andrew Lunn]

This is the first patchset implementing support for the mv88e6390
family.  This is a new generation of switch devices and has numerous
incompatible changes to the registers. These patches allow the switch
to the detected during probe, and makes the statistics unit work.

These patches are insufficient to make the mv88e6390 functional. More
patches will follow.

Andrew Lunn (11):
  net: dsa: mv88e6xxx: Take switch out of reset before probe
  net: dsa: mv88e6xxx: Fix unused variable warning by using variable
  net: dsa: mv88e6xxx: Add the mv88e6390 family
  net: dsa: mv88e6xxx: Abstract stats_snapshot into ops structure
  net: dsa: mv88e6xxx: Add comment about family a device belongs to
  net: dsa: mv88e6xxx: Add mv88e6390 stats snapshot operation
  net: dsa: mv88e6xxx: Add mv88e6390 statistics unit init
  net: dsa: mv88e6xxx: Add stats_get_sset_count to ops structure
  net: dsa: mv88e6xxx: Add stats_get_strings to ops structure
  net: dsa: mv88e6xxx: Add stats_get_stats to ops structure
  net: dsa: mv88e6xxx: Implement mv88e6390 get_stats

 .../devicetree/bindings/net/dsa/marvell.txt        |   3 +-
 drivers/net/dsa/mv88e6xxx/chip.c                   | 550 ++++++++++++++++-----
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h              |  53 +-
 3 files changed, 487 insertions(+), 119 deletions(-)

-- 
2.10.1

[PATCH net-next 01/11] net: dsa: mv88e6xxx: Take switch out of reset before probe

[Andrew Lunn]

The switch needs to be taken out of reset before we can read its ID
register on the MDIO bus.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d6d9d66b81ce..2c5c37318e31 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3839,16 +3839,16 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
 	if (err)
 		return err;
 
+	chip->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(chip->reset))
+		return PTR_ERR(chip->reset);
+
 	err = mv88e6xxx_detect(chip);
 	if (err)
 		return err;
 
 	mv88e6xxx_phy_init(chip);
 
-	chip->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
-	if (IS_ERR(chip->reset))
-		return PTR_ERR(chip->reset);
-
 	if (chip->info->ops->get_eeprom &&
 	    !of_property_read_u32(np, "eeprom-length", &eeprom_len))
 		chip->eeprom_len = eeprom_len;
-- 
2.10.2

Re: [PATCH net-next 01/11] net: dsa: mv88e6xxx: Take switch out of reset before probe

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> The switch needs to be taken out of reset before we can read its ID
> register on the MDIO bus.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Thanks,

        Vivien

[PATCH net-next 02/11] net: dsa: mv88e6xxx: Fix unused variable warning by using variable

[Andrew Lunn]

 _mv88e6xxx_stats_wait() did not check the return value from
 mv88e6xxx_g1_read(), so the compiler complained about set but unused
 err.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 2c5c37318e31..a4d52f0fdc90 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -771,6 +771,9 @@ static int _mv88e6xxx_stats_wait(struct mv88e6xxx_chip *chip)
 
 	for (i = 0; i < 10; i++) {
 		err = mv88e6xxx_g1_read(chip, GLOBAL_STATS_OP, &val);
+		if (err)
+			return err;
+
 		if ((val & GLOBAL_STATS_OP_BUSY) == 0)
 			return 0;
 	}
-- 
2.10.2

Re: [PATCH net-next 02/11] net: dsa: mv88e6xxx: Fix unused variable warning by using variable

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

>  _mv88e6xxx_stats_wait() did not check the return value from
>  mv88e6xxx_g1_read(), so the compiler complained about set but unused
>  err.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Thanks,

        Vivien

[PATCH net-next 03/11] net: dsa: mv88e6xxx: Add the mv88e6390 family

[Andrew Lunn]

With the devices added to the tables, the probe will recognize the
switch. This however is not sufficient to make it work properly, other
changes are needed because of incompatibilities.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 .../devicetree/bindings/net/dsa/marvell.txt        |   3 +-
 drivers/net/dsa/mv88e6xxx/chip.c                   | 105 +++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h              |  25 +++++
 3 files changed, 132 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/dsa/marvell.txt b/Documentation/devicetree/bindings/net/dsa/marvell.txt
index 32025eb4b31b..f543483458a1 100644
--- a/Documentation/devicetree/bindings/net/dsa/marvell.txt
+++ b/Documentation/devicetree/bindings/net/dsa/marvell.txt
@@ -14,7 +14,8 @@ The properties described here are those specific to Marvell devices.
 Additional required and optional properties can be found in dsa.txt.
 
 Required properties:
-- compatible           : Should be one of "marvell,mv88e6085",
+- compatible	       : Should be one of "marvell,mv88e6085" or
+			 "marvell,mv88e6390"
 - reg                  : Address on the MII bus for the switch.
 
 Optional properties:
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index a4d52f0fdc90..066d9479c981 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3312,6 +3312,26 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.port_set_speed = mv88e6352_port_set_speed,
 };
 
+static const struct mv88e6xxx_ops mv88e6390_ops = {
+	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
+	.phy_read = mv88e6xxx_g2_smi_phy_read,
+	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.port_set_link = mv88e6xxx_port_set_link,
+	.port_set_duplex = mv88e6xxx_port_set_duplex,
+	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
+	.port_set_speed = mv88e6390_port_set_speed,
+};
+
+static const struct mv88e6xxx_ops mv88e6390x_ops = {
+	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
+	.phy_read = mv88e6xxx_g2_smi_phy_read,
+	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.port_set_link = mv88e6xxx_port_set_link,
+	.port_set_duplex = mv88e6xxx_port_set_duplex,
+	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
+	.port_set_speed = mv88e6390x_port_set_speed,
+};
+
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 	[MV88E6085] = {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6085,
@@ -3467,6 +3487,47 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.ops = &mv88e6185_ops,
 	},
 
+	[MV88E6190] = {
+		.prod_num = PORT_SWITCH_ID_PROD_NUM_6190,
+		.family = MV88E6XXX_FAMILY_6390,
+		.name = "Marvell 88E6190",
+		.num_databases = 4096,
+		.num_ports = 11,	/* 10 + Z80 */
+		.port_base_addr = 0x0,
+		.global1_addr = 0x1b,
+		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
+		.ops = &mv88e6390_ops,
+	},
+
+	[MV88E6190X] = {
+		.prod_num = PORT_SWITCH_ID_PROD_NUM_6190X,
+		.family = MV88E6XXX_FAMILY_6390,
+		.name = "Marvell 88E6190X",
+		.num_databases = 4096,
+		.num_ports = 11,	/* 10 + Z80 */
+		.port_base_addr = 0x0,
+		.global1_addr = 0x1b,
+		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
+		.ops = &mv88e6390x_ops,
+	},
+
+	[MV88E6191] = {
+		.prod_num = PORT_SWITCH_ID_PROD_NUM_6191,
+		.family = MV88E6XXX_FAMILY_6390,
+		.name = "Marvell 88E6191",
+		.num_databases = 4096,
+		.num_ports = 11,	/* 10 + Z80 */
+		.port_base_addr = 0x0,
+		.global1_addr = 0x1b,
+		.age_time_coeff = 15000,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
+		.ops = &mv88e6390_ops,
+	},
+
 	[MV88E6240] = {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6240,
 		.family = MV88E6XXX_FAMILY_6352,
@@ -3481,6 +3542,20 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.ops = &mv88e6240_ops,
 	},
 
+	[MV88E6290] = {
+		.prod_num = PORT_SWITCH_ID_PROD_NUM_6290,
+		.family = MV88E6XXX_FAMILY_6390,
+		.name = "Marvell 88E6290",
+		.num_databases = 4096,
+		.num_ports = 11,	/* 10 + Z80 */
+		.port_base_addr = 0x0,
+		.global1_addr = 0x1b,
+		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
+		.ops = &mv88e6390_ops,
+	},
+
 	[MV88E6320] = {
 		.prod_num = PORT_SWITCH_ID_PROD_NUM_6320,
 		.family = MV88E6XXX_FAMILY_6320,
@@ -3550,6 +3625,32 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.flags = MV88E6XXX_FLAGS_FAMILY_6352,
 		.ops = &mv88e6352_ops,
 	},
+	[MV88E6390] = {
+		.prod_num = PORT_SWITCH_ID_PROD_NUM_6390,
+		.family = MV88E6XXX_FAMILY_6390,
+		.name = "Marvell 88E6390",
+		.num_databases = 4096,
+		.num_ports = 11,	/* 10 + Z80 */
+		.port_base_addr = 0x0,
+		.global1_addr = 0x1b,
+		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
+		.ops = &mv88e6390_ops,
+	},
+	[MV88E6390X] = {
+		.prod_num = PORT_SWITCH_ID_PROD_NUM_6390X,
+		.family = MV88E6XXX_FAMILY_6390,
+		.name = "Marvell 88E6390X",
+		.num_databases = 4096,
+		.num_ports = 11,	/* 10 + Z80 */
+		.port_base_addr = 0x0,
+		.global1_addr = 0x1b,
+		.age_time_coeff = 15000,
+		.g1_irqs = 9,
+		.flags = MV88E6XXX_FLAGS_FAMILY_6390,
+		.ops = &mv88e6390x_ops,
+	},
 };
 
 static const struct mv88e6xxx_info *mv88e6xxx_lookup_info(unsigned int prod_num)
@@ -3927,6 +4028,10 @@ static const struct of_device_id mv88e6xxx_of_match[] = {
 		.compatible = "marvell,mv88e6085",
 		.data = &mv88e6xxx_table[MV88E6085],
 	},
+	{
+		.compatible = "marvell,mv88e6390",
+		.data = &mv88e6xxx_table[MV88E6390],
+	},
 	{ /* sentinel */ },
 };
 
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 929613021eff..48915e5e3a3d 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -91,11 +91,17 @@
 #define PORT_SWITCH_ID_PROD_NUM_6175	0x175
 #define PORT_SWITCH_ID_PROD_NUM_6176	0x176
 #define PORT_SWITCH_ID_PROD_NUM_6185	0x1a7
+#define PORT_SWITCH_ID_PROD_NUM_6190	0x190
+#define PORT_SWITCH_ID_PROD_NUM_6190X	0x0a0
+#define PORT_SWITCH_ID_PROD_NUM_6191	0x191
 #define PORT_SWITCH_ID_PROD_NUM_6240	0x240
+#define PORT_SWITCH_ID_PROD_NUM_6290	0x290
 #define PORT_SWITCH_ID_PROD_NUM_6321	0x310
 #define PORT_SWITCH_ID_PROD_NUM_6352	0x352
 #define PORT_SWITCH_ID_PROD_NUM_6350	0x371
 #define PORT_SWITCH_ID_PROD_NUM_6351	0x375
+#define PORT_SWITCH_ID_PROD_NUM_6390	0x390
+#define PORT_SWITCH_ID_PROD_NUM_6390X	0x0a1
 #define PORT_CONTROL		0x04
 #define PORT_CONTROL_USE_CORE_TAG	BIT(15)
 #define PORT_CONTROL_DROP_ON_LOCK	BIT(14)
@@ -378,12 +384,18 @@ enum mv88e6xxx_model {
 	MV88E6175,
 	MV88E6176,
 	MV88E6185,
+	MV88E6190,
+	MV88E6190X,
+	MV88E6191,
 	MV88E6240,
+	MV88E6290,
 	MV88E6320,
 	MV88E6321,
 	MV88E6350,
 	MV88E6351,
 	MV88E6352,
+	MV88E6390,
+	MV88E6390X,
 };
 
 enum mv88e6xxx_family {
@@ -396,6 +408,7 @@ enum mv88e6xxx_family {
 	MV88E6XXX_FAMILY_6320,	/* 6320 6321 */
 	MV88E6XXX_FAMILY_6351,	/* 6171 6175 6350 6351 */
 	MV88E6XXX_FAMILY_6352,	/* 6172 6176 6240 6352 */
+	MV88E6XXX_FAMILY_6390,  /* 6190 6190X 6191 6290 6390 6390X */
 };
 
 enum mv88e6xxx_cap {
@@ -615,6 +628,18 @@ enum mv88e6xxx_cap {
 
 struct mv88e6xxx_ops;
 
+#define MV88E6XXX_FLAGS_FAMILY_6390	\
+	(MV88E6XXX_FLAG_EEE |		\
+	 MV88E6XXX_FLAG_GLOBAL2 |	\
+	 MV88E6XXX_FLAG_PPU_ACTIVE |	\
+	 MV88E6XXX_FLAG_STU |		\
+	 MV88E6XXX_FLAG_TEMP |		\
+	 MV88E6XXX_FLAG_TEMP_LIMIT |	\
+	 MV88E6XXX_FLAG_VTU |		\
+	 MV88E6XXX_FLAGS_IRL |		\
+	 MV88E6XXX_FLAGS_MULTI_CHIP |	\
+	 MV88E6XXX_FLAGS_PVT)
+
 struct mv88e6xxx_info {
 	enum mv88e6xxx_family family;
 	u16 prod_num;
-- 
2.10.2

Re: [PATCH net-next 03/11] net: dsa: mv88e6xxx: Add the mv88e6390 family

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> -- compatible           : Should be one of "marvell,mv88e6085",
> +- compatible	       : Should be one of "marvell,mv88e6085" or
> +			 "marvell,mv88e6390"

Just curious here, mv88e6085 was choosen because it was the smaller
product ID supported. Following that logic, shouldn't mv88e6190 be
choosen here instead of mv88e6390?

> +static const struct mv88e6xxx_ops mv88e6390_ops = {
> +	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
> +	.phy_read = mv88e6xxx_g2_smi_phy_read,
> +	.phy_write = mv88e6xxx_g2_smi_phy_write,
> +	.port_set_link = mv88e6xxx_port_set_link,
> +	.port_set_duplex = mv88e6xxx_port_set_duplex,
> +	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
> +	.port_set_speed = mv88e6390_port_set_speed,
> +};
> +
> +static const struct mv88e6xxx_ops mv88e6390x_ops = {
> +	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
> +	.phy_read = mv88e6xxx_g2_smi_phy_read,
> +	.phy_write = mv88e6xxx_g2_smi_phy_write,
> +	.port_set_link = mv88e6xxx_port_set_link,
> +	.port_set_duplex = mv88e6xxx_port_set_duplex,
> +	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
> +	.port_set_speed = mv88e6390x_port_set_speed,
> +};

Even if it is a bit more verbose, I'd intentionally keep one
mv88e6xxx_ops structure per chip. Using per-family structure is
error-prone and simpler is better here.

Thanks,

        Vivien

[PATCH net-next 04/11] net: dsa: mv88e6xxx: Abstract stats_snapshot into ops structure

[Andrew Lunn]

Taking a stats snapshot differs between same families. Abstract this
into an ops member.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 37 +++++++++++++++++++++++++++++++----
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  5 +++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 066d9479c981..65f34f1999e7 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -785,9 +785,6 @@ static int _mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
 {
 	int err;
 
-	if (mv88e6xxx_6320_family(chip) || mv88e6xxx_6352_family(chip))
-		port = (port + 1) << 5;
-
 	/* Snapshot the hardware statistics counters for this port. */
 	err = mv88e6xxx_g1_write(chip, GLOBAL_STATS_OP,
 				 GLOBAL_STATS_OP_CAPTURE_PORT |
@@ -799,6 +796,21 @@ static int _mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
 	return _mv88e6xxx_stats_wait(chip);
 }
 
+static int mv88e6320_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
+{
+	port = (port + 1) << 5;
+
+	return _mv88e6xxx_stats_snapshot(chip, port);
+}
+
+static int mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
+{
+	if (!chip->info->ops->stats_snapshot)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->stats_snapshot(chip, port);
+}
+
 static void _mv88e6xxx_stats_read(struct mv88e6xxx_chip *chip,
 				  int stat, u32 *val)
 {
@@ -987,7 +999,7 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 
 	mutex_lock(&chip->reg_lock);
 
-	ret = _mv88e6xxx_stats_snapshot(chip, port);
+	ret = mv88e6xxx_stats_snapshot(chip, port);
 	if (ret < 0) {
 		mutex_unlock(&chip->reg_lock);
 		return;
@@ -3146,6 +3158,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -3155,6 +3168,7 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6123_ops = {
@@ -3164,6 +3178,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
@@ -3173,6 +3188,7 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3182,6 +3198,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
@@ -3191,6 +3208,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
@@ -3201,6 +3219,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -3213,6 +3232,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
+	.stats_snapshot = mv88e6320_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -3223,6 +3243,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -3235,6 +3256,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
+	.stats_snapshot = mv88e6320_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -3244,6 +3266,7 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6240_ops = {
@@ -3256,6 +3279,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
+	.stats_snapshot = mv88e6320_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
@@ -3267,6 +3291,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = mv88e6320_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
@@ -3278,6 +3303,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = mv88e6320_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
@@ -3288,6 +3314,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -3298,6 +3325,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
+	.stats_snapshot = mv88e6xxx_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -3310,6 +3338,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
+	.stats_snapshot = mv88e6320_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 48915e5e3a3d..fd5352bf7625 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -793,6 +793,11 @@ struct mv88e6xxx_ops {
 	 * Use SPEED_UNFORCED for normal detection, SPEED_MAX for max value.
 	 */
 	int (*port_set_speed)(struct mv88e6xxx_chip *chip, int port, int speed);
+
+	/* Snapshot the statistics for a port. The statistics can then
+	 * be read back a leisure but still with a consistent view.
+	 */
+	int (*stats_snapshot)(struct mv88e6xxx_chip *chip, int port);
 };
 
 enum stat_type {
-- 
2.10.2

Re: [PATCH net-next 04/11] net: dsa: mv88e6xxx: Abstract stats_snapshot into ops structure

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> +static int mv88e6320_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
> +{
> +	port = (port + 1) << 5;
> +
> +	return _mv88e6xxx_stats_snapshot(chip, port);
> +}

Please move the above helper in its internal SMI file (port, global1 or
whatever) and keep the below wrapper in chip.c. The correct prefix will
avoid having a _ prefix.

> +static int mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
> +{
> +	if (!chip->info->ops->stats_snapshot)
> +		return -EOPNOTSUPP;
> +
> +	return chip->info->ops->stats_snapshot(chip, port);
> +}

[...]

>  static const struct mv88e6xxx_ops mv88e6175_ops = {
> @@ -3223,6 +3243,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
>  	.port_set_duplex = mv88e6xxx_port_set_duplex,
>  	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
>  	.port_set_speed = mv88e6185_port_set_speed,
> +	.stats_snapshot = mv88e6xxx_stats_snapshot,
>  };

Is this expected? Doesn't look correct to me to use
mv88e6xxx_stats_snapshot here.

Thanks,

        Vivien

[PATCH net-next 05/11] net: dsa: mv88e6xxx: Add comment about family a device belongs to

[Andrew Lunn]

Knowing the family of device belongs to helps with picking the ops
implementation which is appropriate to the device. So add a comment to
each structure of ops.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 65f34f1999e7..d4fc12d7021a 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3152,6 +3152,7 @@ static int mv88e6xxx_set_eeprom(struct dsa_switch *ds,
 }
 
 static const struct mv88e6xxx_ops mv88e6085_ops = {
+	/* MV88E6XXX_FAMILY_6097 */
 	.set_switch_mac = mv88e6xxx_g1_set_switch_mac,
 	.phy_read = mv88e6xxx_phy_ppu_read,
 	.phy_write = mv88e6xxx_phy_ppu_write,
@@ -3162,6 +3163,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
+	/* MV88E6XXX_FAMILY_6095 */
 	.set_switch_mac = mv88e6xxx_g1_set_switch_mac,
 	.phy_read = mv88e6xxx_phy_ppu_read,
 	.phy_write = mv88e6xxx_phy_ppu_write,
@@ -3172,6 +3174,7 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6123_ops = {
+	/* MV88E6XXX_FAMILY_6165 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_read,
 	.phy_write = mv88e6xxx_write,
@@ -3182,6 +3185,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
+	/* MV88E6XXX_FAMILY_6185 */
 	.set_switch_mac = mv88e6xxx_g1_set_switch_mac,
 	.phy_read = mv88e6xxx_phy_ppu_read,
 	.phy_write = mv88e6xxx_phy_ppu_write,
@@ -3192,6 +3196,7 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
+	/* MV88E6XXX_FAMILY_6165 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_read,
 	.phy_write = mv88e6xxx_write,
@@ -3202,6 +3207,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
+	/* MV88E6XXX_FAMILY_6165 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_read,
 	.phy_write = mv88e6xxx_write,
@@ -3212,6 +3218,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
+	/* MV88E6XXX_FAMILY_6351 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_g2_smi_phy_read,
 	.phy_write = mv88e6xxx_g2_smi_phy_write,
@@ -3223,6 +3230,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
+	/* MV88E6XXX_FAMILY_6352 */
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
@@ -3236,6 +3244,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
+	/* MV88E6XXX_FAMILY_6351 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_g2_smi_phy_read,
 	.phy_write = mv88e6xxx_g2_smi_phy_write,
@@ -3247,6 +3256,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
+	/* MV88E6XXX_FAMILY_6352 */
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
@@ -3260,6 +3270,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
+	/* MV88E6XXX_FAMILY_6185 */
 	.set_switch_mac = mv88e6xxx_g1_set_switch_mac,
 	.phy_read = mv88e6xxx_phy_ppu_read,
 	.phy_write = mv88e6xxx_phy_ppu_write,
@@ -3270,6 +3281,7 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6240_ops = {
+	/* MV88E6XXX_FAMILY_6352 */
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
@@ -3283,6 +3295,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
+	/* MV88E6XXX_FAMILY_6320 */
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
@@ -3295,6 +3308,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
+	/* MV88E6XXX_FAMILY_6321 */
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
@@ -3307,6 +3321,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
+	/* MV88E6XXX_FAMILY_6351 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_g2_smi_phy_read,
 	.phy_write = mv88e6xxx_g2_smi_phy_write,
@@ -3318,6 +3333,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
+	/* MV88E6XXX_FAMILY_6351 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_g2_smi_phy_read,
 	.phy_write = mv88e6xxx_g2_smi_phy_write,
@@ -3329,6 +3345,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
+	/* MV88E6XXX_FAMILY_6352 */
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
@@ -3342,6 +3359,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
+	/* MV88E6XXX_FAMILY_6390 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_g2_smi_phy_read,
 	.phy_write = mv88e6xxx_g2_smi_phy_write,
@@ -3352,6 +3370,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
+	/* MV88E6XXX_FAMILY_6390 */
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
 	.phy_read = mv88e6xxx_g2_smi_phy_read,
 	.phy_write = mv88e6xxx_g2_smi_phy_write,
-- 
2.10.2

Re: [PATCH net-next 05/11] net: dsa: mv88e6xxx: Add comment about family a device belongs to

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> Knowing the family of device belongs to helps with picking the ops
> implementation which is appropriate to the device. So add a comment to
> each structure of ops.

This commit is not necessary. mv88e6xxx_ops structure must be per-chip,
and the family information is already described in patch 03/11.

Thanks,

        Vivien

Re: [PATCH net-next 05/11] net: dsa: mv88e6xxx: Add comment about family a device belongs to

[Andrew Lunn]

On Mon, Nov 14, 2016 at 01:08:13PM +1100, Vivien Didelot wrote:
> Hi Andrew,
> 
> Andrew Lunn <andrew@lunn.ch> writes:
> 
> > Knowing the family of device belongs to helps with picking the ops
> > implementation which is appropriate to the device. So add a comment to
> > each structure of ops.
> 
> This commit is not necessary. mv88e6xxx_ops structure must be per-chip,
> and the family information is already described in patch 03/11.

I disagree. I made a lot of errors adding the right per family handler
to these structures, simply because it is not obvious what family a
device belongs to when looking at the structure.

       Andrew

[PATCH net-next 06/11] net: dsa: mv88e6xxx: Add mv88e6390 stats snapshot operation

[Andrew Lunn]

The MV88E6390 has a control register for what the histogram statistics
actually contain. This means the stat_snapshot method should not set
this information. So implement the 6390 stats_snapshot function without
these bits.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 18 ++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  4 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d4fc12d7021a..82e57aac00f1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -803,6 +803,22 @@ static int mv88e6320_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
 	return _mv88e6xxx_stats_snapshot(chip, port);
 }
 
+static int mv88e6390_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
+{
+	int err;
+
+	port = (port + 1) << 5;
+
+	/* Snapshot the hardware statistics counters for this port. */
+	err = mv88e6xxx_g1_write(chip, GLOBAL_STATS_OP,
+				 GLOBAL_STATS_OP_CAPTURE_PORT | port);
+	if (err)
+		return err;
+
+	/* Wait for the snapshotting to complete. */
+	return _mv88e6xxx_stats_wait(chip);
+}
+
 static int mv88e6xxx_stats_snapshot(struct mv88e6xxx_chip *chip, int port)
 {
 	if (!chip->info->ops->stats_snapshot)
@@ -3367,6 +3383,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
 	.port_set_speed = mv88e6390_port_set_speed,
+	.stats_snapshot = mv88e6390_stats_snapshot,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -3378,6 +3395,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
 	.port_set_speed = mv88e6390x_port_set_speed,
+	.stats_snapshot = mv88e6390_stats_snapshot,
 };
 
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index fd5352bf7625..147f1f3a817b 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -283,7 +283,9 @@
 #define GLOBAL_CONTROL_2	0x1c
 #define GLOBAL_CONTROL_2_NO_CASCADE		0xe000
 #define GLOBAL_CONTROL_2_MULTIPLE_CASCADE	0xf000
-
+#define GLOBAL_CONTROL_2_HIST_RX	       (0x1 << 6)
+#define GLOBAL_CONTROL_2_HIST_TX	       (0x2 << 6)
+#define GLOBAL_CONTROL_2_HIST_RX_TX	       (0x3 << 6)
 #define GLOBAL_STATS_OP		0x1d
 #define GLOBAL_STATS_OP_BUSY	BIT(15)
 #define GLOBAL_STATS_OP_NOP		(0 << 12)
-- 
2.10.2

[PATCH net-next 07/11] net: dsa: mv88e6xxx: Add mv88e6390 statistics unit init

[Andrew Lunn]

The statistics unit on the mv88e6390 needs to the configured in a
different register to the others as to what histogram statistics is
should return.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 31 +++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 82e57aac00f1..b20732616031 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1031,6 +1031,30 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 	mutex_unlock(&chip->reg_lock);
 }
 
+static int mv88e6390_stats_init(struct mv88e6xxx_chip *chip)
+{
+	u16 val;
+	int err;
+
+	err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL_2, &val);
+	if (err)
+		return err;
+
+	val |= GLOBAL_CONTROL_2_HIST_RX_TX;
+
+	err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL_2, val);
+
+	return err;
+}
+
+static int mv88e6xxx_stats_init(struct mv88e6xxx_chip *chip)
+{
+	if (chip->info->ops->stats_init)
+		return chip->info->ops->stats_init(chip);
+
+	return 0;
+}
+
 static int mv88e6xxx_get_regs_len(struct dsa_switch *ds, int port)
 {
 	return 32 * sizeof(u16);
@@ -2819,6 +2843,11 @@ static int mv88e6xxx_g1_setup(struct mv88e6xxx_chip *chip)
 	if (err)
 		return err;
 
+	/* Initialize the statistics unit */
+	err = mv88e6xxx_stats_init(chip);
+	if (err)
+		return err;
+
 	/* Clear the statistics counters for all ports */
 	err = mv88e6xxx_g1_write(chip, GLOBAL_STATS_OP,
 				 GLOBAL_STATS_OP_FLUSH_ALL);
@@ -3383,6 +3412,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
 	.port_set_speed = mv88e6390_port_set_speed,
+	.stats_init = mv88e6390_stats_init,
 	.stats_snapshot = mv88e6390_stats_snapshot,
 };
 
@@ -3395,6 +3425,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
 	.port_set_speed = mv88e6390x_port_set_speed,
+	.stats_init = mv88e6390_stats_init,
 	.stats_snapshot = mv88e6390_stats_snapshot,
 };
 
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 147f1f3a817b..86dbdc957f28 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -796,6 +796,8 @@ struct mv88e6xxx_ops {
 	 */
 	int (*port_set_speed)(struct mv88e6xxx_chip *chip, int port, int speed);
 
+	int (*stats_init)(struct mv88e6xxx_chip *chip);
+
 	/* Snapshot the statistics for a port. The statistics can then
 	 * be read back a leisure but still with a consistent view.
 	 */
-- 
2.10.2

Re: [PATCH net-next 07/11] net: dsa: mv88e6xxx: Add mv88e6390 statistics unit init

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> The statistics unit on the mv88e6390 needs to the configured in a
> different register to the others as to what histogram statistics is
> should return.

Can you re-phrase the above please?

> +static int mv88e6390_stats_init(struct mv88e6xxx_chip *chip)
> +{
> +	u16 val;
> +	int err;
> +
> +	err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL_2, &val);
> +	if (err)
> +		return err;
> +
> +	val |= GLOBAL_CONTROL_2_HIST_RX_TX;
> +
> +	err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL_2, val);
> +
> +	return err;
> +}

Can you please move this Global 1 specific helper in global1.c under an
ordered snippet such as:

    /* Offset 0x1C: Global Control 2 */

    int mv88e6xxx_g1_set_foo(struct mv88e6xxx_chip *chip)
    {
        ...
    }

I'd like internal SMI devices to be self documented in their specific
files and easy to hack for new developers. Ordered helpers will help.

Also, the helper should reflect what it really does. It is used to set
the Histogram Counters Mode. So please name it accordingly, something
like mv88e6xxx_g1_set_hist_count_mode().

Thanks,

        Vivien

[PATCH net-next 08/11] net: dsa: mv88e6xxx: Add stats_get_sset_count to ops structure

[Andrew Lunn]

Different families have different sets of statistics. Abstract this
using a stats_get_sset_count op. Each stat has a bitmap, and the ops
implementer uses a bit map mask to count the statistics which apply
for the family.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 185 +++++++++++++++++++++-------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  11 +-
 2 files changed, 119 insertions(+), 77 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b20732616031..5d2c135b9175 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -860,76 +860,76 @@ static void _mv88e6xxx_stats_read(struct mv88e6xxx_chip *chip,
 }
 
 static struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
-	{ "in_good_octets",	8, 0x00, BANK0, },
-	{ "in_bad_octets",	4, 0x02, BANK0, },
-	{ "in_unicast",		4, 0x04, BANK0, },
-	{ "in_broadcasts",	4, 0x06, BANK0, },
-	{ "in_multicasts",	4, 0x07, BANK0, },
-	{ "in_pause",		4, 0x16, BANK0, },
-	{ "in_undersize",	4, 0x18, BANK0, },
-	{ "in_fragments",	4, 0x19, BANK0, },
-	{ "in_oversize",	4, 0x1a, BANK0, },
-	{ "in_jabber",		4, 0x1b, BANK0, },
-	{ "in_rx_error",	4, 0x1c, BANK0, },
-	{ "in_fcs_error",	4, 0x1d, BANK0, },
-	{ "out_octets",		8, 0x0e, BANK0, },
-	{ "out_unicast",	4, 0x10, BANK0, },
-	{ "out_broadcasts",	4, 0x13, BANK0, },
-	{ "out_multicasts",	4, 0x12, BANK0, },
-	{ "out_pause",		4, 0x15, BANK0, },
-	{ "excessive",		4, 0x11, BANK0, },
-	{ "collisions",		4, 0x1e, BANK0, },
-	{ "deferred",		4, 0x05, BANK0, },
-	{ "single",		4, 0x14, BANK0, },
-	{ "multiple",		4, 0x17, BANK0, },
-	{ "out_fcs_error",	4, 0x03, BANK0, },
-	{ "late",		4, 0x1f, BANK0, },
-	{ "hist_64bytes",	4, 0x08, BANK0, },
-	{ "hist_65_127bytes",	4, 0x09, BANK0, },
-	{ "hist_128_255bytes",	4, 0x0a, BANK0, },
-	{ "hist_256_511bytes",	4, 0x0b, BANK0, },
-	{ "hist_512_1023bytes", 4, 0x0c, BANK0, },
-	{ "hist_1024_max_bytes", 4, 0x0d, BANK0, },
-	{ "sw_in_discards",	4, 0x10, PORT, },
-	{ "sw_in_filtered",	2, 0x12, PORT, },
-	{ "sw_out_filtered",	2, 0x13, PORT, },
-	{ "in_discards",	4, 0x00 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_filtered",	4, 0x01 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_accepted",	4, 0x02 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_bad_accepted",	4, 0x03 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_good_avb_class_a", 4, 0x04 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_good_avb_class_b", 4, 0x05 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_bad_avb_class_a", 4, 0x06 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_bad_avb_class_b", 4, 0x07 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "tcam_counter_0",	4, 0x08 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "tcam_counter_1",	4, 0x09 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "tcam_counter_2",	4, 0x0a | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "tcam_counter_3",	4, 0x0b | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_da_unknown",	4, 0x0e | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "in_management",	4, 0x0f | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_0",	4, 0x10 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_1",	4, 0x11 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_2",	4, 0x12 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_3",	4, 0x13 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_4",	4, 0x14 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_5",	4, 0x15 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_6",	4, 0x16 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_queue_7",	4, 0x17 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_cut_through",	4, 0x18 | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_octets_a",	4, 0x1a | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_octets_b",	4, 0x1b | GLOBAL_STATS_OP_BANK_1, BANK1, },
-	{ "out_management",	4, 0x1f | GLOBAL_STATS_OP_BANK_1, BANK1, },
+	{ "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, },
 };
 
 static bool mv88e6xxx_has_stat(struct mv88e6xxx_chip *chip,
 			       struct mv88e6xxx_hw_stat *stat)
 {
 	switch (stat->type) {
-	case BANK0:
+	case STATS_TYPE_BANK0:
 		return true;
-	case BANK1:
+	case STATS_TYPE_BANK1:
 		return mv88e6xxx_6320_family(chip);
-	case PORT:
+	case STATS_TYPE_PORT:
 		return mv88e6xxx_6095_family(chip) ||
 			mv88e6xxx_6185_family(chip) ||
 			mv88e6xxx_6097_family(chip) ||
@@ -946,12 +946,12 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
 {
 	u32 low;
 	u32 high = 0;
+	u16 reg = 0;
 	int err;
-	u16 reg;
 	u64 value;
 
 	switch (s->type) {
-	case PORT:
+	case STATS_TYPE_PORT:
 		err = mv88e6xxx_port_read(chip, port, s->reg, &reg);
 		if (err)
 			return UINT64_MAX;
@@ -964,11 +964,14 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
 			high = reg;
 		}
 		break;
-	case BANK0:
-	case BANK1:
-		_mv88e6xxx_stats_read(chip, s->reg, &low);
+	case STATS_TYPE_BANK1:
+		reg = GLOBAL_STATS_OP_BANK_1;
+		/* fall through */
+	case STATS_TYPE_BANK0:
+		reg |= s->reg;
+		_mv88e6xxx_stats_read(chip, reg, &low);
 		if (s->sizeof_stat == 8)
-			_mv88e6xxx_stats_read(chip, s->reg + 1, &high);
+			_mv88e6xxx_stats_read(chip, reg + 1, &high);
 	}
 	value = (((u64)high) << 16) | low;
 	return value;
@@ -991,20 +994,41 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
 	}
 }
 
-static int mv88e6xxx_get_sset_count(struct dsa_switch *ds)
+static int _mv88e6xxx_get_sset_count(struct mv88e6xxx_chip *chip, int types)
 {
-	struct mv88e6xxx_chip *chip = ds->priv;
 	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];
-		if (mv88e6xxx_has_stat(chip, stat))
+		if (stat->type & types)
 			j++;
 	}
 	return j;
 }
 
+static int mv88e6095_get_sset_count(struct mv88e6xxx_chip *chip)
+{
+	return _mv88e6xxx_get_sset_count(chip, STATS_TYPE_BANK0 |
+					 STATS_TYPE_PORT);
+}
+
+static int mv88e6320_get_sset_count(struct mv88e6xxx_chip *chip)
+{
+	return _mv88e6xxx_get_sset_count(chip, STATS_TYPE_BANK0 |
+					 STATS_TYPE_BANK1);
+}
+
+static int mv88e6xxx_get_sset_count(struct dsa_switch *ds)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+
+	if (chip->info->ops->stats_get_sset_count)
+		return chip->info->ops->stats_get_sset_count(chip);
+
+	return 0;
+}
+
 static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 					uint64_t *data)
 {
@@ -3205,6 +3229,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -3216,6 +3241,7 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6123_ops = {
@@ -3227,6 +3253,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
@@ -3238,6 +3265,7 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3249,6 +3277,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
@@ -3260,6 +3289,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
@@ -3272,6 +3302,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -3286,6 +3317,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -3298,6 +3330,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -3312,6 +3345,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -3323,6 +3357,7 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6240_ops = {
@@ -3337,6 +3372,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
@@ -3350,6 +3386,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
+	.stats_get_sset_count = mv88e6320_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
@@ -3363,6 +3400,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.port_set_duplex = mv88e6xxx_port_set_duplex,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
+	.stats_get_sset_count = mv88e6320_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
@@ -3375,6 +3413,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -3387,6 +3426,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -3401,6 +3441,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
+	.stats_get_sset_count = mv88e6095_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
@@ -3414,6 +3455,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.port_set_speed = mv88e6390_port_set_speed,
 	.stats_init = mv88e6390_stats_init,
 	.stats_snapshot = mv88e6390_stats_snapshot,
+	.stats_get_sset_count = mv88e6320_get_sset_count,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -3427,6 +3469,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.port_set_speed = mv88e6390x_port_set_speed,
 	.stats_init = mv88e6390_stats_init,
 	.stats_snapshot = mv88e6390_stats_snapshot,
+	.stats_get_sset_count = mv88e6320_get_sset_count,
 };
 
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 86dbdc957f28..4ff674d0e248 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -802,19 +802,18 @@ struct mv88e6xxx_ops {
 	 * be read back a leisure but still with a consistent view.
 	 */
 	int (*stats_snapshot)(struct mv88e6xxx_chip *chip, int port);
+	int (*stats_get_sset_count)(struct mv88e6xxx_chip *chip);
 };
 
-enum stat_type {
-	BANK0,
-	BANK1,
-	PORT,
-};
+#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];
 	int sizeof_stat;
 	int reg;
-	enum stat_type type;
+	int type;
 };
 
 static inline bool mv88e6xxx_has(struct mv88e6xxx_chip *chip,
-- 
2.10.2

Re: [PATCH net-next 08/11] net: dsa: mv88e6xxx: Add stats_get_sset_count to ops structure

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> Different families have different sets of statistics. Abstract this
> using a stats_get_sset_count op. Each stat has a bitmap, and the ops
> implementer uses a bit map mask to count the statistics which apply
> for the family.

> -static int mv88e6xxx_get_sset_count(struct dsa_switch *ds)
> +static int _mv88e6xxx_get_sset_count(struct mv88e6xxx_chip *chip, int types)

Looks good overall. But please don't re-introduce underscore-prefixed
helpers. If I'm not mistaken, stats are a Global 1 feature, so ordered
explicit helpers in global1.c will be perfect.

If the stats code is huge, don't hesitate to move them in a
global1_stats.c file, as you wish. But we have to keep it
self-documented and easy to follow for new developers.

Thanks,

        Vivien

[PATCH net-next 09/11] net: dsa: mv88e6xxx: Add stats_get_strings to ops structure

[Andrew Lunn]

Different families have different sets of statistics. Abstract this
using a stats_get_strings op. Each stat has a bitmap, and the ops
implementer uses a bit map mask to return a list of strings for the
statistics which apply for the family.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 47 ++++++++++++++++++++++++++++++++---
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  1 +
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 5d2c135b9175..cfbf0f0ff0dd 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -977,16 +977,15 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
 	return value;
 }
 
-static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
-				  uint8_t *data)
+static void _mv88e6xxx_get_strings(struct mv88e6xxx_chip *chip,
+				   uint8_t *data, int types)
 {
-	struct mv88e6xxx_chip *chip = ds->priv;
 	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];
-		if (mv88e6xxx_has_stat(chip, stat)) {
+		if (stat->type & types) {
 			memcpy(data + j * ETH_GSTRING_LEN, stat->string,
 			       ETH_GSTRING_LEN);
 			j++;
@@ -994,6 +993,27 @@ static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
 	}
 }
 
+static void mv88e6095_get_strings(struct mv88e6xxx_chip *chip, uint8_t *data)
+{
+	_mv88e6xxx_get_strings(chip, data,
+			       STATS_TYPE_BANK0 | STATS_TYPE_PORT);
+}
+
+static void mv88e6320_get_strings(struct mv88e6xxx_chip *chip, uint8_t *data)
+{
+	_mv88e6xxx_get_strings(chip, data,
+			       STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
+}
+
+static void mv88e6xxx_get_strings(struct dsa_switch *ds, int port,
+				  uint8_t *data)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+
+	if (chip->info->ops->stats_get_strings)
+		chip->info->ops->stats_get_strings(chip, data);
+}
+
 static int _mv88e6xxx_get_sset_count(struct mv88e6xxx_chip *chip, int types)
 {
 	struct mv88e6xxx_hw_stat *stat;
@@ -3230,6 +3250,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -3242,6 +3263,7 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6123_ops = {
@@ -3254,6 +3276,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
@@ -3266,6 +3289,7 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3278,6 +3302,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
@@ -3290,6 +3315,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
@@ -3303,6 +3329,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -3318,6 +3345,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -3331,6 +3359,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -3346,6 +3375,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -3358,6 +3388,7 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6240_ops = {
@@ -3373,6 +3404,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
@@ -3387,6 +3419,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
+	.stats_get_strings = mv88e6320_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
@@ -3401,6 +3434,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
+	.stats_get_strings = mv88e6320_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
@@ -3414,6 +3448,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -3427,6 +3462,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.port_set_speed = mv88e6185_port_set_speed,
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -3442,6 +3478,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.port_set_speed = mv88e6352_port_set_speed,
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
+	.stats_get_strings = mv88e6095_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
@@ -3456,6 +3493,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.stats_init = mv88e6390_stats_init,
 	.stats_snapshot = mv88e6390_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
+	.stats_get_strings = mv88e6320_get_strings,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -3470,6 +3508,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.stats_init = mv88e6390_stats_init,
 	.stats_snapshot = mv88e6390_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
+	.stats_get_strings = mv88e6320_get_strings,
 };
 
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index 4ff674d0e248..d3b8eed109a6 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -803,6 +803,7 @@ struct mv88e6xxx_ops {
 	 */
 	int (*stats_snapshot)(struct mv88e6xxx_chip *chip, int port);
 	int (*stats_get_sset_count)(struct mv88e6xxx_chip *chip);
+	void (*stats_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
 };
 
 #define STATS_TYPE_PORT		BIT(0)
-- 
2.10.2

[PATCH net-next 10/11] net: dsa: mv88e6xxx: Add stats_get_stats to ops structure

[Andrew Lunn]

Different families have different sets of statistics. Abstract this
using a stats_get_stats op. The mv88e6390 needs a different
implementation, which will be added later.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 84 +++++++++++++++++++++++------------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  2 +
 2 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index cfbf0f0ff0dd..ebef214eb1e3 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -921,25 +921,6 @@ static struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
 	{ "out_management",		4, 0x1f, STATS_TYPE_BANK1, },
 };
 
-static bool mv88e6xxx_has_stat(struct mv88e6xxx_chip *chip,
-			       struct mv88e6xxx_hw_stat *stat)
-{
-	switch (stat->type) {
-	case STATS_TYPE_BANK0:
-		return true;
-	case STATS_TYPE_BANK1:
-		return mv88e6xxx_6320_family(chip);
-	case STATS_TYPE_PORT:
-		return mv88e6xxx_6095_family(chip) ||
-			mv88e6xxx_6185_family(chip) ||
-			mv88e6xxx_6097_family(chip) ||
-			mv88e6xxx_6165_family(chip) ||
-			mv88e6xxx_6351_family(chip) ||
-			mv88e6xxx_6352_family(chip);
-	}
-	return false;
-}
-
 static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
 					    struct mv88e6xxx_hw_stat *s,
 					    int port)
@@ -1049,13 +1030,48 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds)
 	return 0;
 }
 
+static void _mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
+				 uint64_t *data, int types)
+{
+	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];
+		if (stat->type & types) {
+			data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port,
+							      bank1_select);
+			j++;
+		}
+	}
+}
+
+static void mv88e6095_get_stats(struct mv88e6xxx_chip *chip, int port,
+				uint64_t *data)
+{
+	return _mv88e6xxx_get_stats(chip, port, data,
+				    STATS_TYPE_BANK0 | STATS_TYPE_PORT);
+}
+
+static void mv88e6320_get_stats(struct mv88e6xxx_chip *chip, int port,
+				uint64_t *data)
+{
+	return _mv88e6xxx_get_stats(chip, port, data,
+				    STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
+}
+
+static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
+				uint64_t *data)
+{
+	if (chip->info->ops->stats_get_stats)
+		chip->info->ops->stats_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;
-	struct mv88e6xxx_hw_stat *stat;
 	int ret;
-	int i, j;
 
 	mutex_lock(&chip->reg_lock);
 
@@ -1064,13 +1080,8 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 		mutex_unlock(&chip->reg_lock);
 		return;
 	}
-	for (i = 0, j = 0; i < ARRAY_SIZE(mv88e6xxx_hw_stats); i++) {
-		stat = &mv88e6xxx_hw_stats[i];
-		if (mv88e6xxx_has_stat(chip, stat)) {
-			data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port);
-			j++;
-		}
-	}
+
+	mv88e6xxx_get_stats(chip, port, data);
 
 	mutex_unlock(&chip->reg_lock);
 }
@@ -3251,6 +3262,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -3264,6 +3276,7 @@ static const struct mv88e6xxx_ops mv88e6095_ops = {
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6123_ops = {
@@ -3277,6 +3290,7 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6131_ops = {
@@ -3290,6 +3304,7 @@ static const struct mv88e6xxx_ops mv88e6131_ops = {
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6161_ops = {
@@ -3303,6 +3318,7 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6165_ops = {
@@ -3316,6 +3332,7 @@ static const struct mv88e6xxx_ops mv88e6165_ops = {
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6171_ops = {
@@ -3330,6 +3347,7 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6172_ops = {
@@ -3346,6 +3364,7 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6175_ops = {
@@ -3360,6 +3379,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6176_ops = {
@@ -3376,6 +3396,7 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6185_ops = {
@@ -3389,6 +3410,7 @@ static const struct mv88e6xxx_ops mv88e6185_ops = {
 	.stats_snapshot = _mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6240_ops = {
@@ -3405,6 +3427,7 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
@@ -3420,6 +3443,7 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
 	.stats_get_strings = mv88e6320_get_strings,
+	.stats_get_stats = mv88e6320_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6321_ops = {
@@ -3435,6 +3459,7 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
 	.stats_get_strings = mv88e6320_get_strings,
+	.stats_get_stats = mv88e6320_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6350_ops = {
@@ -3449,6 +3474,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6351_ops = {
@@ -3463,6 +3489,7 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.stats_snapshot = mv88e6xxx_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6352_ops = {
@@ -3479,6 +3506,7 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.stats_snapshot = mv88e6320_stats_snapshot,
 	.stats_get_sset_count = mv88e6095_get_sset_count,
 	.stats_get_strings = mv88e6095_get_strings,
+	.stats_get_stats = mv88e6095_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6390_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index d3b8eed109a6..e0196450e8a2 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -804,6 +804,8 @@ struct mv88e6xxx_ops {
 	int (*stats_snapshot)(struct mv88e6xxx_chip *chip, int port);
 	int (*stats_get_sset_count)(struct mv88e6xxx_chip *chip);
 	void (*stats_get_strings)(struct mv88e6xxx_chip *chip,  uint8_t *data);
+	void (*stats_get_stats)(struct mv88e6xxx_chip *chip,  int port,
+				uint64_t *data);
 };
 
 #define STATS_TYPE_PORT		BIT(0)
-- 
2.10.2

[PATCH net-next 11/11] net: dsa: mv88e6xxx: Implement mv88e6390 get_stats

[Andrew Lunn]

The mv88e6390 uses a different bit to select between bank0 and bank1
of the statistics. So implement an ops function for this, and pass the
selector bit to the generic stats read function. Also, the histogram
selection has moved for the mv88e6390, so abstract its selection as
well.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c      | 33 ++++++++++++++++++++++++---------
 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h |  3 ++-
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ebef214eb1e3..9e3cdc356ae2 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -837,8 +837,7 @@ static void _mv88e6xxx_stats_read(struct mv88e6xxx_chip *chip,
 	*val = 0;
 
 	err = mv88e6xxx_g1_write(chip, GLOBAL_STATS_OP,
-				 GLOBAL_STATS_OP_READ_CAPTURED |
-				 GLOBAL_STATS_OP_HIST_RX_TX | stat);
+				 GLOBAL_STATS_OP_READ_CAPTURED | stat);
 	if (err)
 		return;
 
@@ -923,7 +922,8 @@ static struct mv88e6xxx_hw_stat mv88e6xxx_hw_stats[] = {
 
 static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
 					    struct mv88e6xxx_hw_stat *s,
-					    int port)
+					    int port, u16 bank1_select,
+					    u16 histogram)
 {
 	u32 low;
 	u32 high = 0;
@@ -946,10 +946,10 @@ static uint64_t _mv88e6xxx_get_ethtool_stat(struct mv88e6xxx_chip *chip,
 		}
 		break;
 	case STATS_TYPE_BANK1:
-		reg = GLOBAL_STATS_OP_BANK_1;
+		reg = bank1_select;
 		/* fall through */
 	case STATS_TYPE_BANK0:
-		reg |= s->reg;
+		reg |= s->reg | histogram;
 		_mv88e6xxx_stats_read(chip, reg, &low);
 		if (s->sizeof_stat == 8)
 			_mv88e6xxx_stats_read(chip, reg + 1, &high);
@@ -1031,7 +1031,8 @@ static int mv88e6xxx_get_sset_count(struct dsa_switch *ds)
 }
 
 static void _mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
-				 uint64_t *data, int types)
+				 uint64_t *data, int types, u16 bank1_select,
+				 u16 histogram)
 {
 	struct mv88e6xxx_hw_stat *stat;
 	int i, j;
@@ -1040,7 +1041,8 @@ static void _mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
 		stat = &mv88e6xxx_hw_stats[i];
 		if (stat->type & types) {
 			data[j] = _mv88e6xxx_get_ethtool_stat(chip, stat, port,
-							      bank1_select);
+							      bank1_select,
+							      histogram);
 			j++;
 		}
 	}
@@ -1050,14 +1052,25 @@ static void mv88e6095_get_stats(struct mv88e6xxx_chip *chip, int port,
 				uint64_t *data)
 {
 	return _mv88e6xxx_get_stats(chip, port, data,
-				    STATS_TYPE_BANK0 | STATS_TYPE_PORT);
+				    STATS_TYPE_BANK0 | STATS_TYPE_PORT,
+				    0, GLOBAL_STATS_OP_HIST_RX_TX);
 }
 
 static void mv88e6320_get_stats(struct mv88e6xxx_chip *chip, int port,
 				uint64_t *data)
 {
 	return _mv88e6xxx_get_stats(chip, port, data,
-				    STATS_TYPE_BANK0 | STATS_TYPE_BANK1);
+				    STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
+				    GLOBAL_STATS_OP_BANK_1_BIT_9,
+				    GLOBAL_STATS_OP_HIST_RX_TX);
+}
+
+static void mv88e6390_get_stats(struct mv88e6xxx_chip *chip, int port,
+				uint64_t *data)
+{
+	return _mv88e6xxx_get_stats(chip, port, data,
+				    STATS_TYPE_BANK0 | STATS_TYPE_BANK1,
+				    GLOBAL_STATS_OP_BANK_1_BIT_10, 0);
 }
 
 static void mv88e6xxx_get_stats(struct mv88e6xxx_chip *chip, int port,
@@ -3522,6 +3535,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.stats_snapshot = mv88e6390_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
 	.stats_get_strings = mv88e6320_get_strings,
+	.stats_get_stats = mv88e6390_get_stats,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -3537,6 +3551,7 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.stats_snapshot = mv88e6390_stats_snapshot,
 	.stats_get_sset_count = mv88e6320_get_sset_count,
 	.stats_get_strings = mv88e6320_get_strings,
+	.stats_get_stats = mv88e6390_get_stats,
 };
 
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
diff --git a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
index e0196450e8a2..9ec1fbd94a3a 100644
--- a/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx/mv88e6xxx.h
@@ -296,7 +296,8 @@
 #define GLOBAL_STATS_OP_HIST_RX		((1 << 10) | GLOBAL_STATS_OP_BUSY)
 #define GLOBAL_STATS_OP_HIST_TX		((2 << 10) | GLOBAL_STATS_OP_BUSY)
 #define GLOBAL_STATS_OP_HIST_RX_TX	((3 << 10) | GLOBAL_STATS_OP_BUSY)
-#define GLOBAL_STATS_OP_BANK_1	BIT(9)
+#define GLOBAL_STATS_OP_BANK_1_BIT_9	BIT(9)
+#define GLOBAL_STATS_OP_BANK_1_BIT_10	BIT(10)
 #define GLOBAL_STATS_COUNTER_32	0x1e
 #define GLOBAL_STATS_COUNTER_01	0x1f
 
-- 
2.10.2

Re: [PATCH net-next 00/11] Start adding support for mv88e6390 family

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Fri, 11 Nov 2016 03:53:32 +0100

> This is the first patchset implementing support for the mv88e6390
> family.  This is a new generation of switch devices and has numerous
> incompatible changes to the registers. These patches allow the switch
> to the detected during probe, and makes the statistics unit work.
> 
> These patches are insufficient to make the mv88e6390 functional. More
> patches will follow.

Andrew, this series doesn't apply cleanly to net-next, so you'll
need to respin.

Thanks.

Re: [PATCH net-next 00/11] Start adding support for mv88e6390 family

[Andrew Lunn]

On Sun, Nov 13, 2016 at 12:48:59AM -0500, David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Fri, 11 Nov 2016 03:53:32 +0100
> 
> > This is the first patchset implementing support for the mv88e6390
> > family.  This is a new generation of switch devices and has numerous
> > incompatible changes to the registers. These patches allow the switch
> > to the detected during probe, and makes the statistics unit work.
> > 
> > These patches are insufficient to make the mv88e6390 functional. More
> > patches will follow.
> 
> Andrew, this series doesn't apply cleanly to net-next, so you'll
> need to respin.

Hi David

I'm happy to respin, but i'm wondering why the don't apply.

What seems to be the issue is you said you have accepted:

[PATCH net-next 0/2] Fixes for port refactoring
https://marc.info/?l=linux-netdev&m=147880114928996&w=1

Yet i don't see these in net-next. And i based this patchset on a tree
which included the fixes. Hence they are not applying.

Have the fixes really been accepted?

Thanks
	Andrew


 

Re: [PATCH net-next 00/11] Start adding support for mv88e6390 family

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Sun, 13 Nov 2016 21:24:03 +0100

> What seems to be the issue is you said you have accepted:
> 
> [PATCH net-next 0/2] Fixes for port refactoring
> https://marc.info/?l=linux-netdev&m=147880114928996&w=1
> 
> Yet i don't see these in net-next. And i based this patchset on a tree
> which included the fixes. Hence they are not applying.
> 
> Have the fixes really been accepted?

Accepted but not pushed out properly, sorry.

This should be sorted out now.

Re: [PATCH net-next 00/11] Start adding support for mv88e6390 family

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Sun, 13 Nov 2016 21:24:03 +0100

> I'm happy to respin, but i'm wondering why the don't apply.

Andrew, even though this issue has been resolved, it looks like Vivien
has some feedback for you to address with this series so I guess I'll
see a v2 or similar soon.

Thanks.