Subject: Re: [PATCH net-next v1 1/7] net: ethtool: plumb PHY stats to PHY drivers

From: Simon Horman <[email-masked]>
Date: Tue, 10 Dec 2024 12:03:19 +0000

On Tue, Dec 03, 2024 at 08:56:15AM +0100, Oleksij Rempel wrote:
> From: Jakub Kicinski <[email-masked]>
> 
> Feed the existing IEEE PHY counter struct (which currently
> only has one entry) and link stats into the PHY driver.
> The MAC driver can override the value if it somehow has a better
> idea of PHY stats. Since the stats are "undefined" at input
> the drivers can't += the values, so we should be safe from
> double-counting.
> 
> Signed-off-by: Jakub Kicinski <[email-masked]>
> Signed-off-by: Oleksij Rempel <[email-masked]>
> ---
>  include/linux/phy.h     | 10 ++++++++++
>  net/ethtool/linkstate.c | 25 ++++++++++++++++++++++---
>  net/ethtool/stats.c     | 19 +++++++++++++++++++
>  3 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 563c46205685..523195c724b5 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1090,6 +1090,16 @@ struct phy_driver {
>  	int (*cable_test_get_status)(struct phy_device *dev, bool *finished);
>  
>  	/* Get statistics from the PHY using ethtool */
> +	/**
> +	 * @get_phy_stats: Get well known statistics.
> +	 * @get_link_stats: Get well known link statistics.
> +	 * The input structure is not zero-initialized and the implementation
> +	 * must only set statistics which are actually collected by the device.
> +	 */
> +	void (*get_phy_stats)(struct phy_device *dev,
> +			      struct ethtool_eth_phy_stats *eth_stats);

nit: There should be a kernel doc for @get_link_stats here.

     Flagged by ./scripts/kernel-doc -none

> +	void (*get_link_stats)(struct phy_device *dev,
> +			       struct ethtool_link_ext_stats *link_stats);
>  	/** @get_sset_count: Number of statistic counters */
>  	int (*get_sset_count)(struct phy_device *dev);
>  	/** @get_strings: Names of the statistic counters */

...