Re: [PATCH net-next v2 3/3] phy: dp83tg720: Add statistics support

Hi Maxime,

On Wed, Aug 21, 2024 at 10:16:22AM +0200, Maxime Chevallier wrote:

Hello Oleksij,

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

quoted

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 :

quoted

+/**
+ * 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 ?

You are right, it is outdated. My previous implementation had rgmii-fail
status flags.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |