[net-next][PATCH v2 1/2] dsa: lan9303: Change stat name

[net-next][PATCH v2 1/2] dsa: lan9303: Change stat name

[Jerry Ray]

This patch changes the reported ethtool statistics for the lan9303
family of parts covered by this driver.

The TxUnderRun statistic label is renamed to RxShort to accurately
reflect what stat the device is reporting.  I did not reorder the
statistics as that might cause problems with existing user code that
are expecting the stats at a certain offset.

Signed-off-by: Jerry Ray <jerry.ray@microchip.com>
---
v1->v2:
  Split patch into 2 pieces.
---
 drivers/net/dsa/lan9303-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 438e46af03e9..80f07bd20593 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -961,7 +961,7 @@ static const struct lan9303_mib_desc lan9303_mib[] = {
 	{ .offset = LAN9303_MAC_TX_BRDCST_CNT_0, .name = "TxBroad", },
 	{ .offset = LAN9303_MAC_TX_PAUSE_CNT_0, .name = "TxPause", },
 	{ .offset = LAN9303_MAC_TX_MULCST_CNT_0, .name = "TxMulti", },
-	{ .offset = LAN9303_MAC_RX_UNDSZE_CNT_0, .name = "TxUnderRun", },
+	{ .offset = LAN9303_MAC_RX_UNDSZE_CNT_0, .name = "RxShort", },
 	{ .offset = LAN9303_MAC_TX_64_CNT_0, .name = "Tx64Byte", },
 	{ .offset = LAN9303_MAC_TX_127_CNT_0, .name = "Tx128Byte", },
 	{ .offset = LAN9303_MAC_TX_255_CNT_0, .name = "Tx256Byte", },
-- 
2.17.1

[net-next][PATCH v2 2/2] dsa: lan9303: Add 2 ethtool stats

[Jerry Ray]

Adding RxDropped and TxDropped counters to the reported statistics.
As these stats are kept by the switch rather than the port instance,
they are indexed differently.

Signed-off-by: Jerry Ray <jerry.ray@microchip.com>
---
v1->v2:
  Split patch into 2 pieces.
  Removed the adding of a module number to the driver.
---
 drivers/net/dsa/lan9303-core.c | 38 ++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c
index 80f07bd20593..714a21d7aa0a 100644
--- a/drivers/net/dsa/lan9303-core.c
+++ b/drivers/net/dsa/lan9303-core.c
@@ -177,10 +177,12 @@
 #define LAN9303_SWE_INGRESS_PORT_TYPE 0x1847
 #define  LAN9303_SWE_INGRESS_PORT_TYPE_VLAN 3
 #define LAN9303_BM_CFG 0x1c00
+#define LAN9303_BM_DRP_CNT_SRC_0 0x1c05
 #define LAN9303_BM_EGRSS_PORT_TYPE 0x1c0c
 # define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT2 (BIT(17) | BIT(16))
 # define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT1 (BIT(9) | BIT(8))
 # define LAN9303_BM_EGRSS_PORT_TYPE_SPECIAL_TAG_PORT0 (BIT(1) | BIT(0))
+#define LAN9303_BM_RATE_DRP_CNT_SRC_0 0x1c16
 
 #define LAN9303_SWITCH_PORT_REG(port, reg0) (0x400 * (port) + (reg0))
 
@@ -978,10 +980,16 @@ static const struct lan9303_mib_desc lan9303_mib[] = {
 	{ .offset = LAN9303_MAC_TX_LATECOL_0, .name = "TxLateCol", },
 };
 
+/* Buffer Management Statistics (indexed by port) */
+static const struct lan9303_mib_desc lan9303_switch_mib[] = {
+	{ .offset = LAN9303_BM_RATE_DRP_CNT_SRC_0, .name = "RxDropped", },
+	{ .offset = LAN9303_BM_DRP_CNT_SRC_0, .name = "TxDropped", },
+};
+
 static void lan9303_get_strings(struct dsa_switch *ds, int port,
 				u32 stringset, uint8_t *data)
 {
-	unsigned int u;
+	unsigned int i, u;
 
 	if (stringset != ETH_SS_STATS)
 		return;
@@ -990,26 +998,44 @@ static void lan9303_get_strings(struct dsa_switch *ds, int port,
 		strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name,
 			ETH_GSTRING_LEN);
 	}
+	for (i = 0; i < ARRAY_SIZE(lan9303_switch_mib); i++) {
+		strncpy(data + (u + i) * ETH_GSTRING_LEN,
+			lan9303_switch_mib[i].name, ETH_GSTRING_LEN);
+	}
 }
 
 static void lan9303_get_ethtool_stats(struct dsa_switch *ds, int port,
 				      uint64_t *data)
 {
 	struct lan9303 *chip = ds->priv;
-	unsigned int u;
+	unsigned int i, u;
 
 	for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) {
 		u32 reg;
 		int ret;
 
-		ret = lan9303_read_switch_port(
-			chip, port, lan9303_mib[u].offset, &reg);
-
+		/* Read Port-based MIB stats. */
+		ret = lan9303_read_switch_port(chip, port,
+					       lan9303_mib[u].offset,
+					       &reg);
 		if (ret)
 			dev_warn(chip->dev, "Reading status port %d reg %u failed\n",
 				 port, lan9303_mib[u].offset);
 		data[u] = reg;
 	}
+	for (i = 0; i < ARRAY_SIZE(lan9303_switch_mib); i++) {
+		u32 reg;
+		int ret;
+
+		/* Read Switch stats indexed by port. */
+		ret = lan9303_read_switch_reg(chip,
+					      (lan9303_switch_mib[i].offset +
+					       port), &reg);
+		if (ret)
+			dev_warn(chip->dev, "Reading status port %d reg %u failed\n",
+				 port, lan9303_switch_mib[i].offset);
+		data[i + u] = reg;
+	}
 }
 
 static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset)
@@ -1017,7 +1043,7 @@ static int lan9303_get_sset_count(struct dsa_switch *ds, int port, int sset)
 	if (sset != ETH_SS_STATS)
 		return 0;
 
-	return ARRAY_SIZE(lan9303_mib);
+	return ARRAY_SIZE(lan9303_mib) + ARRAY_SIZE(lan9303_switch_mib);
 }
 
 static int lan9303_phy_read(struct dsa_switch *ds, int phy, int regnum)
-- 
2.17.1

Re: [net-next][PATCH v2 2/2] dsa: lan9303: Add 2 ethtool stats

[Andrew Lunn]

On Tue, Nov 15, 2022 at 10:51:31AM -0600, Jerry Ray wrote:
> Adding RxDropped and TxDropped counters to the reported statistics.
> As these stats are kept by the switch rather than the port instance,
> they are indexed differently.
> 
> Signed-off-by: Jerry Ray <jerry.ray@microchip.com>

Hi Jerry

Your Subject line is not as we would like. For your next patchset,
please take a look at the netdev FAQ. You should also have a patch 0/X
explaining the big picture.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
	   

Re: [net-next][PATCH v2 2/2] dsa: lan9303: Add 2 ethtool stats

[Jakub Kicinski]

On Tue, 15 Nov 2022 10:51:31 -0600 Jerry Ray wrote:
> Adding RxDropped and TxDropped counters to the reported statistics.
> As these stats are kept by the switch rather than the port instance,
> they are indexed differently.

What are these statistics counting? They are still per port, so should
they be reported in struct rtnl_link_stats::[rt]x_dropped ?

Re: [net-next][PATCH v2 1/2] dsa: lan9303: Change stat name

[Andrew Lunn]

On Tue, Nov 15, 2022 at 10:51:30AM -0600, Jerry Ray wrote:
> This patch changes the reported ethtool statistics for the lan9303
> family of parts covered by this driver.
> 
> The TxUnderRun statistic label is renamed to RxShort to accurately
> reflect what stat the device is reporting.  I did not reorder the
> statistics as that might cause problems with existing user code that
> are expecting the stats at a certain offset.
> 
> Signed-off-by: Jerry Ray <jerry.ray@microchip.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next][PATCH v2 1/2] dsa: lan9303: Change stat name

[Florian Fainelli]

On 11/15/2022 8:51 AM, Jerry Ray wrote:
> This patch changes the reported ethtool statistics for the lan9303
> family of parts covered by this driver.
> 
> The TxUnderRun statistic label is renamed to RxShort to accurately
> reflect what stat the device is reporting.  I did not reorder the
> statistics as that might cause problems with existing user code that
> are expecting the stats at a certain offset.
> 
> Signed-off-by: Jerry Ray <jerry.ray@microchip.com>

This looks like a bugfix no? We should not we slap a:

Fixes: a1292595e006 ("net: dsa: add new DSA switch driver for the 
SMSC-LAN9303")

Reviewed-by: Florian Fainelli <f.faineli@gmail.com>
-- 
Florian

Re: [net-next][PATCH v2 1/2] dsa: lan9303: Change stat name

[Jakub Kicinski]

On Wed, 16 Nov 2022 09:08:58 -0800 Florian Fainelli wrote:
> This looks like a bugfix no? We should not we slap a:
> 
> Fixes: a1292595e006 ("net: dsa: add new DSA switch driver for the 
> SMSC-LAN9303")

With the entire Fixes tag as a single line ;) And that'll mean that
patches 1 and 2 need to be posted separately. First one to net and the
second one to net-next.