Hello Oleksij,

On Tue, 20 Aug 2024 14:29:14 +0200
Oleksij Rempel [off-list ref] wrote:

Introduce statistics support for the DP83TG720 PHY driver, enabling
detailed monitoring and reporting of link quality and packet-related
metrics.

To avoid double reading of certain registers, the implementation caches
all relevant register values in a single operation. This approach
ensures accurate and consistent data retrieval, particularly for
registers that clear upon reading or require special handling.

Some of the statistics, such as link training times, do not increment
and therefore require special handling during the extraction process.

This all looks good to me, I do have one small nit bellow :

+/**
+ * dp83tg720_get_stats - Get the statistics values.
+ * @phydev: Pointer to the phy_device structure.
+ * @stats: Pointer to the ethtool_stats structure.
+ * @data: Pointer to the buffer where the statistics values will be stored.
+ *
+ * Fills the buffer with the statistics values, filtering out those that are
+ * not applicable based on the PHY's operating mode (e.g., RGMII).

I don't see how this filtering is actually implemented, is this comment
correct ?

+ */
+static void dp83tg720_get_stats(struct phy_device *phydev,
+				struct ethtool_stats *stats, u64 *data)
+{
+	int i, j = 0;
+
+	dp83tg720_cache_reg_values(phydev);
+
+	for (i = 0; i < ARRAY_SIZE(dp83tg720_hw_stats); i++) {
+		data[j] = dp83tg720_extract_stat_value(phydev, i);
+		j++;
+	}
+}

Thanks,

Maxime