On 2022-08-18 17:36, Andrew Lunn wrote:

quoted

@@ -1310,16 +1323,22 @@ static void mv88e6xxx_get_ethtool_stats(struct dsa_switch *ds, int port,
 	struct mv88e6xxx_chip *chip = ds->priv;
 	int ret;
 
-	mv88e6xxx_reg_lock(chip);
+	if (chip->rmu.ops && chip->rmu.ops->get_rmon) {
+		ret = chip->rmu.ops->get_rmon(chip, port, data);
+		if (ret == -ETIMEDOUT)
+			return;
+	} else {
 
-	ret = mv88e6xxx_stats_snapshot(chip, port);
-	mv88e6xxx_reg_unlock(chip);
+		mv88e6xxx_reg_lock(chip);
 
-	if (ret < 0)
-		return;
+		ret = mv88e6xxx_stats_snapshot(chip, port);
+		mv88e6xxx_reg_unlock(chip);
 
-	mv88e6xxx_get_stats(chip, port, data);
+		if (ret < 0)
+			return;
 
+		mv88e6xxx_get_stats(chip, port, data);
+	}
 }

I don't particularly like the way this is all mixed together.

Could you try to split it, so there is an MDIO set of functions and an
RMU set of functions. Maybe you have some helpers which are used by
both.

I think that a great idea. Will split in separate functions.

I would also suggest you try to think about ATU dump and VTU dump. You
ideally want a code structure which is very similar for all these dump
operations. Take a look at how qca8k-8xxx.c does things.

I think that the ATU dump is the logical next step to implement. I'll
have a look in qca8k to see how they do things.

AFAIK there is no RMU operation for VTU dump. That needs to be done 
with register reads and writes.

Is it documented in the datasheet that when RMU is used a snapshot is
not required?

    Andrew

In the documentation they only mention captured counters with regards 
to MDIO accesses. I cannot see that that is necessary with RMU.
FYI on our board the time delta between request and reply 
is ~100us (which is nice)