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

Luo Jie posted 3 patches 2 months, 3 weeks ago
[PATCH net-next v3 1/3] net: phy: qcom: Add PHY counter support
Posted by Luo Jie 2 months, 3 weeks ago
Add PHY counter functionality to the shared library. The implementation
is identical for the current QCA807X and QCA808X PHYs.

The PHY counter can be configured to perform CRC checking for both received
and transmitted packets. Additionally, the packet counter can be set to
automatically clear after it is read.

The PHY counter includes 32-bit packet counters for both RX (received) and
TX (transmitted) packets, as well as 16-bit counters for recording CRC
error packets for both RX and TX.

Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
---
 drivers/net/phy/qcom/qcom-phy-lib.c | 75 +++++++++++++++++++++++++++++++++++++
 drivers/net/phy/qcom/qcom.h         | 23 ++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/drivers/net/phy/qcom/qcom-phy-lib.c b/drivers/net/phy/qcom/qcom-phy-lib.c
index af7d0d8e81be..965c2bb99a9b 100644
--- a/drivers/net/phy/qcom/qcom-phy-lib.c
+++ b/drivers/net/phy/qcom/qcom-phy-lib.c
@@ -699,3 +699,78 @@ int qca808x_led_reg_blink_set(struct phy_device *phydev, u16 reg,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qca808x_led_reg_blink_set);
+
+/* Enable CRC checking for both received and transmitted frames to ensure
+ * accurate counter recording. The hardware supports a 32-bit counter,
+ * configure the counter to clear after it is read to facilitate the
+ * implementation of a 64-bit software counter
+ */
+int qcom_phy_counter_config(struct phy_device *phydev)
+{
+	return phy_set_bits_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_CTRL,
+				QCA808X_MMD7_CNT_CTRL_CRC_CHECK_EN |
+				QCA808X_MMD7_CNT_CTRL_READ_CLEAR_EN);
+}
+EXPORT_SYMBOL_GPL(qcom_phy_counter_config);
+
+int qcom_phy_update_stats(struct phy_device *phydev,
+			  struct qcom_phy_hw_stats *hw_stats)
+{
+	int ret;
+	u32 cnt;
+
+	/* PHY 32-bit counter for RX packets. */
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_15_0);
+	if (ret < 0)
+		return ret;
+
+	cnt = ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_31_16);
+	if (ret < 0)
+		return ret;
+
+	cnt |= ret << 16;
+	hw_stats->rx_pkts += cnt;
+
+	/* PHY 16-bit counter for RX CRC error packets. */
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_ERR_PKT);
+	if (ret < 0)
+		return ret;
+
+	hw_stats->rx_err_pkts += ret;
+
+	/* PHY 32-bit counter for TX packets. */
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_TX_PKT_15_0);
+	if (ret < 0)
+		return ret;
+
+	cnt = ret;
+
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_TX_PKT_31_16);
+	if (ret < 0)
+		return ret;
+
+	cnt |= ret << 16;
+	hw_stats->tx_pkts += cnt;
+
+	/* PHY 16-bit counter for TX CRC error packets. */
+	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_TX_ERR_PKT);
+	if (ret < 0)
+		return ret;
+
+	hw_stats->tx_err_pkts += ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_phy_update_stats);
+
+void qcom_phy_get_stats(struct ethtool_phy_stats *stats,
+			struct qcom_phy_hw_stats hw_stats)
+{
+	stats->tx_packets = hw_stats.tx_pkts;
+	stats->tx_errors = hw_stats.tx_err_pkts;
+	stats->rx_packets = hw_stats.rx_pkts;
+	stats->rx_errors = hw_stats.rx_err_pkts;
+}
+EXPORT_SYMBOL_GPL(qcom_phy_get_stats);
diff --git a/drivers/net/phy/qcom/qcom.h b/drivers/net/phy/qcom/qcom.h
index 7f7151c8baca..5071e7149a11 100644
--- a/drivers/net/phy/qcom/qcom.h
+++ b/drivers/net/phy/qcom/qcom.h
@@ -195,6 +195,17 @@
 #define AT803X_MIN_DOWNSHIFT			2
 #define AT803X_MAX_DOWNSHIFT			9
 
+#define QCA808X_MMD7_CNT_CTRL			0x8029
+#define QCA808X_MMD7_CNT_CTRL_READ_CLEAR_EN	BIT(1)
+#define QCA808X_MMD7_CNT_CTRL_CRC_CHECK_EN	BIT(0)
+
+#define QCA808X_MMD7_CNT_RX_PKT_31_16		0x802a
+#define QCA808X_MMD7_CNT_RX_PKT_15_0		0x802b
+#define QCA808X_MMD7_CNT_RX_ERR_PKT		0x802c
+#define QCA808X_MMD7_CNT_TX_PKT_31_16		0x802d
+#define QCA808X_MMD7_CNT_TX_PKT_15_0		0x802e
+#define QCA808X_MMD7_CNT_TX_ERR_PKT		0x802f
+
 enum stat_access_type {
 	PHY,
 	MMD
@@ -212,6 +223,13 @@ struct at803x_ss_mask {
 	u8 speed_shift;
 };
 
+struct qcom_phy_hw_stats {
+	u64 rx_pkts;
+	u64 rx_err_pkts;
+	u64 tx_pkts;
+	u64 tx_err_pkts;
+};
+
 int at803x_debug_reg_read(struct phy_device *phydev, u16 reg);
 int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
 			  u16 clear, u16 set);
@@ -246,3 +264,8 @@ int qca808x_led_reg_brightness_set(struct phy_device *phydev,
 int qca808x_led_reg_blink_set(struct phy_device *phydev, u16 reg,
 			      unsigned long *delay_on,
 			      unsigned long *delay_off);
+int qcom_phy_counter_config(struct phy_device *phydev);
+int qcom_phy_update_stats(struct phy_device *phydev,
+			  struct qcom_phy_hw_stats *hw_stats);
+void qcom_phy_get_stats(struct ethtool_phy_stats *stats,
+			struct qcom_phy_hw_stats hw_stats);

-- 
2.34.1
Re: [PATCH net-next v3 1/3] net: phy: qcom: Add PHY counter support
Posted by Andrew Lunn 2 months, 3 weeks ago
On Tue, Jul 15, 2025 at 07:02:26PM +0800, Luo Jie wrote:
> Add PHY counter functionality to the shared library. The implementation
> is identical for the current QCA807X and QCA808X PHYs.
> 
> The PHY counter can be configured to perform CRC checking for both received
> and transmitted packets. Additionally, the packet counter can be set to
> automatically clear after it is read.
> 
> The PHY counter includes 32-bit packet counters for both RX (received) and
> TX (transmitted) packets, as well as 16-bit counters for recording CRC
> error packets for both RX and TX.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Re: [PATCH net-next v3 1/3] net: phy: qcom: Add PHY counter support
Posted by Andrew Lunn 2 months, 3 weeks ago
> +int qcom_phy_update_stats(struct phy_device *phydev,
> +			  struct qcom_phy_hw_stats *hw_stats)
> +{
> +	int ret;
> +	u32 cnt;
> +
> +	/* PHY 32-bit counter for RX packets. */
> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_15_0);
> +	if (ret < 0)
> +		return ret;
> +
> +	cnt = ret;
> +
> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_31_16);
> +	if (ret < 0)
> +		return ret;

Does reading QCA808X_MMD7_CNT_RX_PKT_15_0 cause
QCA808X_MMD7_CNT_RX_PKT_31_16 to latch?

Sometimes you need to read the high part, the low part, and then
reread the high part to ensure it has not incremented. But this is
only needed if the hardware does not latch.

	Andrew
Re: [PATCH net-next v3 1/3] net: phy: qcom: Add PHY counter support
Posted by Luo Jie 2 months, 3 weeks ago

On 7/16/2025 12:11 AM, Andrew Lunn wrote:
>> +int qcom_phy_update_stats(struct phy_device *phydev,
>> +			  struct qcom_phy_hw_stats *hw_stats)
>> +{
>> +	int ret;
>> +	u32 cnt;
>> +
>> +	/* PHY 32-bit counter for RX packets. */
>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_15_0);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	cnt = ret;
>> +
>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_31_16);
>> +	if (ret < 0)
>> +		return ret;
> 
> Does reading QCA808X_MMD7_CNT_RX_PKT_15_0 cause
> QCA808X_MMD7_CNT_RX_PKT_31_16 to latch?

Checked with the hardware design team: The high 16-bit counter register
does not latch when reading the low 16 bits.

> 
> Sometimes you need to read the high part, the low part, and then
> reread the high part to ensure it has not incremented. But this is
> only needed if the hardware does not latch.
> 
> 	Andrew

Since the counter is configured to clear after reading, the clear action
takes priority over latching the count. This means that when reading the
low 16 bits, the high 16-bit counter value cannot increment, any new
packet events occurring during the read will be recorded after the
16-bit counter is cleared.

Therefore, the current sequence for reading the counter is correct and
will not result in missed increments.
Re: [PATCH net-next v3 1/3] net: phy: qcom: Add PHY counter support
Posted by Paolo Abeni 2 months, 3 weeks ago
On 7/16/25 12:15 PM, Luo Jie wrote:
> On 7/16/2025 12:11 AM, Andrew Lunn wrote:
>>> +int qcom_phy_update_stats(struct phy_device *phydev,
>>> +			  struct qcom_phy_hw_stats *hw_stats)
>>> +{
>>> +	int ret;
>>> +	u32 cnt;
>>> +
>>> +	/* PHY 32-bit counter for RX packets. */
>>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_15_0);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	cnt = ret;
>>> +
>>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_31_16);
>>> +	if (ret < 0)
>>> +		return ret;
>>
>> Does reading QCA808X_MMD7_CNT_RX_PKT_15_0 cause
>> QCA808X_MMD7_CNT_RX_PKT_31_16 to latch?
> 
> Checked with the hardware design team: The high 16-bit counter register
> does not latch when reading the low 16 bits.
> 
>>
>> Sometimes you need to read the high part, the low part, and then
>> reread the high part to ensure it has not incremented. But this is
>> only needed if the hardware does not latch.
>>
>> 	Andrew
> 
> Since the counter is configured to clear after reading, the clear action
> takes priority over latching the count. This means that when reading the
> low 16 bits, the high 16-bit counter value cannot increment, any new
> packet events occurring during the read will be recorded after the
> 16-bit counter is cleared.

Out of sheer ignorance and language bias on my side, based on the above
I would have assumed that the registers do latch ;)

> Therefore, the current sequence for reading the counter is correct and
> will not result in missed increments.

Andrew, looks good?

/P
Re: [PATCH net-next v3 1/3] net: phy: qcom: Add PHY counter support
Posted by Andrew Lunn 2 months, 3 weeks ago
On Thu, Jul 17, 2025 at 03:23:16PM +0200, Paolo Abeni wrote:
> On 7/16/25 12:15 PM, Luo Jie wrote:
> > On 7/16/2025 12:11 AM, Andrew Lunn wrote:
> >>> +int qcom_phy_update_stats(struct phy_device *phydev,
> >>> +			  struct qcom_phy_hw_stats *hw_stats)
> >>> +{
> >>> +	int ret;
> >>> +	u32 cnt;
> >>> +
> >>> +	/* PHY 32-bit counter for RX packets. */
> >>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_15_0);
> >>> +	if (ret < 0)
> >>> +		return ret;
> >>> +
> >>> +	cnt = ret;
> >>> +
> >>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_31_16);
> >>> +	if (ret < 0)
> >>> +		return ret;
> >>
> >> Does reading QCA808X_MMD7_CNT_RX_PKT_15_0 cause
> >> QCA808X_MMD7_CNT_RX_PKT_31_16 to latch?
> > 
> > Checked with the hardware design team: The high 16-bit counter register
> > does not latch when reading the low 16 bits.
> > 
> >>
> >> Sometimes you need to read the high part, the low part, and then
> >> reread the high part to ensure it has not incremented. But this is
> >> only needed if the hardware does not latch.
> >>
> >> 	Andrew
> > 
> > Since the counter is configured to clear after reading, the clear action
> > takes priority over latching the count. This means that when reading the
> > low 16 bits, the high 16-bit counter value cannot increment, any new
> > packet events occurring during the read will be recorded after the
> > 16-bit counter is cleared.
> 
> Out of sheer ignorance and language bias on my side, based on the above
> I would have assumed that the registers do latch ;)

I interpret it differently. The register is set to clear on read. So
you read and clear the least significant word. Even if that word
starts incriminating, you have 65535 increments before it will
overflow into the next word. So you can read the most significant word
before such an overflow happens. It does not latch, you just have a
time window when it is safe.

What i actually find odd is that clear on read works on words, not the
full counter. I assume that is documented in the datasheet, and
tested, because i've never seen hardware do that before.

	Andrew
Re: [PATCH net-next v3 1/3] net: phy: qcom: Add PHY counter support
Posted by Luo Jie 2 months, 2 weeks ago

On 7/17/2025 9:46 PM, Andrew Lunn wrote:
> On Thu, Jul 17, 2025 at 03:23:16PM +0200, Paolo Abeni wrote:
>> On 7/16/25 12:15 PM, Luo Jie wrote:
>>> On 7/16/2025 12:11 AM, Andrew Lunn wrote:
>>>>> +int qcom_phy_update_stats(struct phy_device *phydev,
>>>>> +			  struct qcom_phy_hw_stats *hw_stats)
>>>>> +{
>>>>> +	int ret;
>>>>> +	u32 cnt;
>>>>> +
>>>>> +	/* PHY 32-bit counter for RX packets. */
>>>>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_15_0);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>> +
>>>>> +	cnt = ret;
>>>>> +
>>>>> +	ret = phy_read_mmd(phydev, MDIO_MMD_AN, QCA808X_MMD7_CNT_RX_PKT_31_16);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>
>>>> Does reading QCA808X_MMD7_CNT_RX_PKT_15_0 cause
>>>> QCA808X_MMD7_CNT_RX_PKT_31_16 to latch?
>>>
>>> Checked with the hardware design team: The high 16-bit counter register
>>> does not latch when reading the low 16 bits.
>>>
>>>>
>>>> Sometimes you need to read the high part, the low part, and then
>>>> reread the high part to ensure it has not incremented. But this is
>>>> only needed if the hardware does not latch.
>>>>
>>>> 	Andrew
>>>
>>> Since the counter is configured to clear after reading, the clear action
>>> takes priority over latching the count. This means that when reading the
>>> low 16 bits, the high 16-bit counter value cannot increment, any new
>>> packet events occurring during the read will be recorded after the
>>> 16-bit counter is cleared.
>>
>> Out of sheer ignorance and language bias on my side, based on the above
>> I would have assumed that the registers do latch ;)
> 
> I interpret it differently. The register is set to clear on read. So
> you read and clear the least significant word. Even if that word
> starts incriminating, you have 65535 increments before it will
> overflow into the next word. So you can read the most significant word
> before such an overflow happens. It does not latch, you just have a
> time window when it is safe.
> 
> What i actually find odd is that clear on read works on words, not the
> full counter. I assume that is documented in the datasheet, and
> tested, because i've never seen hardware do that before.
> 
> 	Andrew

Thank you for the review. The PHY counter functionality is also used in
the downstream code, and this patch series has been validated
accordingly. However, please note that the PHY counter is intended as a
debug feature and may not be documented in the datasheet. I will share
this feedback with the hardware team in the hope that we can include
documentation for this feature in the datasheet for future chips.