Re: [PATCH net-next 1/3] net: phy: qcom: Add PHY counter support

[Luo Jie <quic_luoj@quicinc.com>]

On 7/9/2025 12:31 AM, Andrew Lunn wrote:
>> +static const struct qcom_phy_hw_stat qcom_phy_hw_stats[] = {
>> +	{
>> +		.string		= "phy_rx_good_frame",
>> +		.devad		= MDIO_MMD_AN,
>> +		.cnt_31_16_reg	= QCA808X_MMD7_CNT_RX_GOOD_CRC_31_16,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_RX_GOOD_CRC_15_0,
>> +	},
>> +	{
>> +		.string		= "phy_rx_bad_frame",
>> +		.devad		= MDIO_MMD_AN,
>> +		.cnt_31_16_reg	= 0xffff,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_RX_BAD_CRC,
>> +	},
>> +	{
>> +		.string		= "phy_tx_good_frame",
>> +		.devad		= MDIO_MMD_AN,
>> +		.cnt_31_16_reg	= QCA808X_MMD7_CNT_TX_GOOD_CRC_31_16,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_TX_GOOD_CRC_15_0,
>> +	},
>> +	{
>> +		.string		= "phy_tx_bad_frame",
>> +		.devad		= MDIO_MMD_AN,
> 
> Are there any counters which might be added later which are not in
> MDIO_MMD_AN? It seems pointless having this if it is fixed.

Yes, this structure is designed to be extended in the future to support
10G-capable PHY chip, where the counter resides in the MDIO_MMD_PCS
when the link speed is 2500 Mbps or higher. I will add a comment to the
structure to clarify this intent.

> 
>> +		.cnt_31_16_reg	= 0xffff,
>> +		.cnt_15_0_reg	= QCA808X_MMD7_CNT_TX_BAD_CRC,
>> +	},
>> +};
> 
> There has been an attempt to try to standardise PHY statistics. Please
> look at:
> 
> **
>   * struct ethtool_phy_stats - PHY-level statistics counters
>   * @rx_packets: Total successfully received frames
>   * @rx_bytes: Total successfully received bytes
>   * @rx_errors: Total received frames with errors (e.g., CRC errors)
>   * @tx_packets: Total successfully transmitted frames
>   * @tx_bytes: Total successfully transmitted bytes
>   * @tx_errors: Total transmitted frames with errors
>   *
>   * This structure provides a standardized interface for reporting
>   * PHY-level statistics counters. It is designed to expose statistics
>   * commonly provided by PHYs but not explicitly defined in the IEEE
>   * 802.3 standard.
>   */
> struct ethtool_phy_stats {
>          u64 rx_packets;
>          u64 rx_bytes;
>          u64 rx_errors;
>          u64 tx_packets;
>          u64 tx_bytes;
>          u64 tx_errors;
> };
> 
> Please use this if possible.
> 
>      Andrew

Thank you for the suggestion. I will review the standardized PHY
statistics framework and update the implementation accordingly to
ensure alignment.

> 
> ---
> pw-bot: cr