Commit a8d00668 authored by Paolo Abeni's avatar Paolo Abeni
Browse files

Merge branch 'introduce-unified-and-structured-phy'

Oleksij Rempel says:

====================
Introduce unified and structured PHY

This patch set introduces a unified and well-structured interface for
reporting PHY statistics. Instead of relying on arbitrary strings in PHY
drivers, this interface provides a consistent and structured way to
expose PHY statistics to userspace via ethtool.

The initial groundwork for this effort was laid by Jakub Kicinski, who
contributed patches to plumb PHY statistics to drivers and added support
for structured statistics in ethtool. Building on Jakub's work, I tested
the implementation with several PHYs, addressed a few issues, and added
support for statistics in two specific PHY drivers.

Most of changes are tracked in separate patches.
changes v6:
- drop ethtool_stat_add patch
changes v5:
- rebase against latest net-next
====================

Link: https://patch.msgid.link/20250110060517.711683-1-o.rempel@pengutronix.de


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents 6a46e3e8 677d895a
Loading
Loading
Loading
Loading
+28 −11
Original line number Diff line number Diff line
@@ -713,17 +713,23 @@ driver supports reporting such events.

- **Monitor Error Counters**:

  - While some NIC drivers and PHYs provide error counters, there is no unified
    set of PHY-specific counters across all hardware. Additionally, not all
    PHYs provide useful information related to errors like CRC errors, frame
    drops, or link flaps. Therefore, this step is dependent on the specific
    hardware and driver support.

  - **Next Steps**: Use `ethtool -S <interface>` to check if your driver
    provides useful error counters. In some cases, counters may provide
    information about errors like link flaps or physical layer problems (e.g.,
    excessive CRC errors), but results can vary significantly depending on the
    PHY.
  - Use `ethtool -S <interface> --all-groups` to retrieve standardized interface
    statistics if the driver supports the unified interface:

  - **Command:** `ethtool -S <interface> --all-groups`

  - **Example Output (if supported)**:

    .. code-block:: bash

      phydev-RxFrames: 100391
      phydev-RxErrors: 0
      phydev-TxFrames: 9
      phydev-TxErrors: 0

  - If the unified interface is not supported, use `ethtool -S <interface>` to
    retrieve MAC and PHY counters. Note that non-standardized PHY counter names
    vary by driver and must be interpreted accordingly:

  - **Command:** `ethtool -S <interface>`

@@ -740,6 +746,17 @@ driver supports reporting such events.
    condition) or kernel log messages (e.g., link up/down events) to further
    diagnose the issue.

  - **Compare Counters**:

    - Compare the egress and ingress frame counts reported by the PHY and MAC.

    - A small difference may occur due to sampling rate differences between the
      MAC and PHY drivers, or if the PHY and MAC are not always fully
      synchronized in their UP or DOWN states.

    - Significant discrepancies indicate potential issues in the data path
      between the MAC and PHY.

When All Else Fails...
~~~~~~~~~~~~~~~~~~~~~~

+1 −0
Original line number Diff line number Diff line
@@ -1616,6 +1616,7 @@ the ``ETHTOOL_A_STATS_GROUPS`` bitset. Currently defined values are:
 ETHTOOL_STATS_ETH_PHY  eth-phy  Basic IEEE 802.3 PHY statistics (30.3.2.1.*)
 ETHTOOL_STATS_ETH_CTRL eth-ctrl Basic IEEE 802.3 MAC Ctrl statistics (30.3.3.*)
 ETHTOOL_STATS_RMON     rmon     RMON (RFC 2819) statistics
 ETHTOOL_STATS_PHY      phy      Additional PHY statistics, not defined by IEEE
 ====================== ======== ===============================================

Each group should have a corresponding ``ETHTOOL_A_STATS_GRP`` in the reply.
+112 −0
Original line number Diff line number Diff line
@@ -34,6 +34,29 @@
#define DP83TD510E_CTRL_HW_RESET		BIT(15)
#define DP83TD510E_CTRL_SW_RESET		BIT(14)

/*
 * DP83TD510E_PKT_STAT_x registers correspond to similarly named registers
 * in the datasheet (PKT_STAT_1 through PKT_STAT_6). These registers store
 * 32-bit or 16-bit counters for TX and RX statistics and must be read in
 * sequence to ensure the counters are cleared correctly.
 *
 * - DP83TD510E_PKT_STAT_1: Contains TX packet count bits [15:0].
 * - DP83TD510E_PKT_STAT_2: Contains TX packet count bits [31:16].
 * - DP83TD510E_PKT_STAT_3: Contains TX error packet count.
 * - DP83TD510E_PKT_STAT_4: Contains RX packet count bits [15:0].
 * - DP83TD510E_PKT_STAT_5: Contains RX packet count bits [31:16].
 * - DP83TD510E_PKT_STAT_6: Contains RX error packet count.
 *
 * Keeping the register names as defined in the datasheet helps maintain
 * clarity and alignment with the documentation.
 */
#define DP83TD510E_PKT_STAT_1			0x12b
#define DP83TD510E_PKT_STAT_2			0x12c
#define DP83TD510E_PKT_STAT_3			0x12d
#define DP83TD510E_PKT_STAT_4			0x12e
#define DP83TD510E_PKT_STAT_5			0x12f
#define DP83TD510E_PKT_STAT_6			0x130

#define DP83TD510E_AN_STAT_1			0x60c
#define DP83TD510E_MASTER_SLAVE_RESOL_FAIL	BIT(15)

@@ -58,8 +81,16 @@ static const u16 dp83td510_mse_sqi_map[] = {
	0x0000  /* 24dB =< SNR */
};

struct dp83td510_stats {
	u64 tx_pkt_cnt;
	u64 tx_err_pkt_cnt;
	u64 rx_pkt_cnt;
	u64 rx_err_pkt_cnt;
};

struct dp83td510_priv {
	bool alcd_test_active;
	struct dp83td510_stats stats;
};

/* Time Domain Reflectometry (TDR) Functionality of DP83TD510 PHY
@@ -177,6 +208,85 @@ struct dp83td510_priv {
#define DP83TD510E_ALCD_COMPLETE			BIT(15)
#define DP83TD510E_ALCD_CABLE_LENGTH			GENMASK(10, 0)

/**
 * dp83td510_update_stats - Update the PHY statistics for the DP83TD510 PHY.
 * @phydev: Pointer to the phy_device structure.
 *
 * The function reads the PHY statistics registers and updates the statistics
 * structure.
 *
 * Returns: 0 on success or a negative error code on failure.
 */
static int dp83td510_update_stats(struct phy_device *phydev)
{
	struct dp83td510_priv *priv = phydev->priv;
	u32 count;
	int ret;

	/* The DP83TD510E_PKT_STAT registers are divided into two groups:
	 * - Group 1 (TX stats): DP83TD510E_PKT_STAT_1 to DP83TD510E_PKT_STAT_3
	 * - Group 2 (RX stats): DP83TD510E_PKT_STAT_4 to DP83TD510E_PKT_STAT_6
	 *
	 * Registers in each group are cleared only after reading them in a
	 * plain sequence (e.g., 1, 2, 3 for Group 1 or 4, 5, 6 for Group 2).
	 * Any deviation from the sequence, such as reading 1, 2, 1, 2, 3, will
	 * prevent the group from being cleared. Additionally, the counters
	 * for a group are frozen as soon as the first register in that group
	 * is accessed.
	 */
	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_PKT_STAT_1);
	if (ret < 0)
		return ret;
	/* tx_pkt_cnt_15_0 */
	count = ret;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_PKT_STAT_2);
	if (ret < 0)
		return ret;
	/* tx_pkt_cnt_31_16 */
	count |= ret << 16;
	priv->stats.tx_pkt_cnt += count;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_PKT_STAT_3);
	if (ret < 0)
		return ret;
	/* tx_err_pkt_cnt */
	priv->stats.tx_err_pkt_cnt += ret;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_PKT_STAT_4);
	if (ret < 0)
		return ret;
	/* rx_pkt_cnt_15_0 */
	count = ret;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_PKT_STAT_5);
	if (ret < 0)
		return ret;
	/* rx_pkt_cnt_31_16 */
	count |= ret << 16;
	priv->stats.rx_pkt_cnt += count;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TD510E_PKT_STAT_6);
	if (ret < 0)
		return ret;
	/* rx_err_pkt_cnt */
	priv->stats.rx_err_pkt_cnt += ret;

	return 0;
}

static void dp83td510_get_phy_stats(struct phy_device *phydev,
				    struct ethtool_eth_phy_stats *eth_stats,
				    struct ethtool_phy_stats *stats)
{
	struct dp83td510_priv *priv = phydev->priv;

	stats->tx_packets = priv->stats.tx_pkt_cnt;
	stats->tx_errors = priv->stats.tx_err_pkt_cnt;
	stats->rx_packets = priv->stats.rx_pkt_cnt;
	stats->rx_errors = priv->stats.rx_err_pkt_cnt;
}

static int dp83td510_config_intr(struct phy_device *phydev)
{
	int ret;
@@ -599,6 +709,8 @@ static struct phy_driver dp83td510_driver[] = {
	.get_sqi_max	= dp83td510_get_sqi_max,
	.cable_test_start = dp83td510_cable_test_start,
	.cable_test_get_status = dp83td510_cable_test_get_status,
	.get_phy_stats	= dp83td510_get_phy_stats,
	.update_stats	= dp83td510_update_stats,

	.suspend	= genphy_suspend,
	.resume		= genphy_resume,
+161 −0
Original line number Diff line number Diff line
@@ -51,6 +51,9 @@
/* Register 0x0405: Unknown Register */
#define DP83TG720S_UNKNOWN_0405			0x405

#define DP83TG720S_LINK_QUAL_3			0x547
#define DP83TG720S_LINK_LOSS_CNT_MASK		GENMASK(15, 10)

/* Register 0x0576: TDR Master Link Down Control */
#define DP83TG720S_TDR_MASTER_LINK_DOWN		0x576

@@ -60,6 +63,29 @@
/* In RGMII mode, Enable or disable the internal delay for TXD */
#define DP83TG720S_RGMII_TX_CLK_SEL		BIT(0)

/*
 * DP83TG720S_PKT_STAT_x registers correspond to similarly named registers
 * in the datasheet (PKT_STAT_1 through PKT_STAT_6). These registers store
 * 32-bit or 16-bit counters for TX and RX statistics and must be read in
 * sequence to ensure the counters are cleared correctly.
 *
 * - DP83TG720S_PKT_STAT_1: Contains TX packet count bits [15:0].
 * - DP83TG720S_PKT_STAT_2: Contains TX packet count bits [31:16].
 * - DP83TG720S_PKT_STAT_3: Contains TX error packet count.
 * - DP83TG720S_PKT_STAT_4: Contains RX packet count bits [15:0].
 * - DP83TG720S_PKT_STAT_5: Contains RX packet count bits [31:16].
 * - DP83TG720S_PKT_STAT_6: Contains RX error packet count.
 *
 * Keeping the register names as defined in the datasheet helps maintain
 * clarity and alignment with the documentation.
 */
#define DP83TG720S_PKT_STAT_1			0x639
#define DP83TG720S_PKT_STAT_2			0x63a
#define DP83TG720S_PKT_STAT_3			0x63b
#define DP83TG720S_PKT_STAT_4			0x63c
#define DP83TG720S_PKT_STAT_5			0x63d
#define DP83TG720S_PKT_STAT_6			0x63e

/* Register 0x083F: Unknown Register */
#define DP83TG720S_UNKNOWN_083F			0x83f

@@ -69,6 +95,113 @@

#define DP83TG720_SQI_MAX			7

struct dp83tg720_stats {
	u64 link_loss_cnt;
	u64 tx_pkt_cnt;
	u64 tx_err_pkt_cnt;
	u64 rx_pkt_cnt;
	u64 rx_err_pkt_cnt;
};

struct dp83tg720_priv {
	struct dp83tg720_stats stats;
};

/**
 * dp83tg720_update_stats - Update the PHY statistics for the DP83TD510 PHY.
 * @phydev: Pointer to the phy_device structure.
 *
 * The function reads the PHY statistics registers and updates the statistics
 * structure.
 *
 * Returns: 0 on success or a negative error code on failure.
 */
static int dp83tg720_update_stats(struct phy_device *phydev)
{
	struct dp83tg720_priv *priv = phydev->priv;
	u32 count;
	int ret;

	/* Read the link loss count */
	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_LINK_QUAL_3);
	if (ret < 0)
		return ret;
	/* link_loss_cnt */
	count = FIELD_GET(DP83TG720S_LINK_LOSS_CNT_MASK, ret);
	priv->stats.link_loss_cnt += count;

	/* The DP83TG720S_PKT_STAT registers are divided into two groups:
	 * - Group 1 (TX stats): DP83TG720S_PKT_STAT_1 to DP83TG720S_PKT_STAT_3
	 * - Group 2 (RX stats): DP83TG720S_PKT_STAT_4 to DP83TG720S_PKT_STAT_6
	 *
	 * Registers in each group are cleared only after reading them in a
	 * plain sequence (e.g., 1, 2, 3 for Group 1 or 4, 5, 6 for Group 2).
	 * Any deviation from the sequence, such as reading 1, 2, 1, 2, 3, will
	 * prevent the group from being cleared. Additionally, the counters
	 * for a group are frozen as soon as the first register in that group
	 * is accessed.
	 */
	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_PKT_STAT_1);
	if (ret < 0)
		return ret;
	/* tx_pkt_cnt_15_0 */
	count = ret;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_PKT_STAT_2);
	if (ret < 0)
		return ret;
	/* tx_pkt_cnt_31_16 */
	count |= ret << 16;
	priv->stats.tx_pkt_cnt += count;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_PKT_STAT_3);
	if (ret < 0)
		return ret;
	/* tx_err_pkt_cnt */
	priv->stats.tx_err_pkt_cnt += ret;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_PKT_STAT_4);
	if (ret < 0)
		return ret;
	/* rx_pkt_cnt_15_0 */
	count = ret;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_PKT_STAT_5);
	if (ret < 0)
		return ret;
	/* rx_pkt_cnt_31_16 */
	count |= ret << 16;
	priv->stats.rx_pkt_cnt += count;

	ret = phy_read_mmd(phydev, MDIO_MMD_VEND2, DP83TG720S_PKT_STAT_6);
	if (ret < 0)
		return ret;
	/* rx_err_pkt_cnt */
	priv->stats.rx_err_pkt_cnt += ret;

	return 0;
}

static void dp83tg720_get_link_stats(struct phy_device *phydev,
				     struct ethtool_link_ext_stats *link_stats)
{
	struct dp83tg720_priv *priv = phydev->priv;

	link_stats->link_down_events = priv->stats.link_loss_cnt;
}

static void dp83tg720_get_phy_stats(struct phy_device *phydev,
				    struct ethtool_eth_phy_stats *eth_stats,
				    struct ethtool_phy_stats *stats)
{
	struct dp83tg720_priv *priv = phydev->priv;

	stats->tx_packets = priv->stats.tx_pkt_cnt;
	stats->tx_errors = priv->stats.tx_err_pkt_cnt;
	stats->rx_packets = priv->stats.rx_pkt_cnt;
	stats->rx_errors = priv->stats.rx_err_pkt_cnt;
}

/**
 * dp83tg720_cable_test_start - Start the cable test for the DP83TG720 PHY.
 * @phydev: Pointer to the phy_device structure.
@@ -182,6 +315,11 @@ static int dp83tg720_cable_test_get_status(struct phy_device *phydev,

	ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A, stat);

	/* save the current stats before resetting the PHY */
	ret = dp83tg720_update_stats(phydev);
	if (ret)
		return ret;

	return phy_init_hw(phydev);
}

@@ -217,6 +355,11 @@ static int dp83tg720_read_status(struct phy_device *phydev)
	phy_sts = phy_read(phydev, DP83TG720S_MII_REG_10);
	phydev->link = !!(phy_sts & DP83TG720S_LINK_STATUS);
	if (!phydev->link) {
		/* save the current stats before resetting the PHY */
		ret = dp83tg720_update_stats(phydev);
		if (ret)
			return ret;

		/* According to the "DP83TC81x, DP83TG72x Software
		 * Implementation Guide", the PHY needs to be reset after a
		 * link loss or if no link is created after at least 100ms.
@@ -341,12 +484,27 @@ static int dp83tg720_config_init(struct phy_device *phydev)
	return genphy_c45_pma_baset1_read_master_slave(phydev);
}

static int dp83tg720_probe(struct phy_device *phydev)
{
	struct device *dev = &phydev->mdio.dev;
	struct dp83tg720_priv *priv;

	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
	if (!priv)
		return -ENOMEM;

	phydev->priv = priv;

	return 0;
}

static struct phy_driver dp83tg720_driver[] = {
{
	PHY_ID_MATCH_MODEL(DP83TG720S_PHY_ID),
	.name		= "TI DP83TG720S",

	.flags          = PHY_POLL_CABLE_TEST,
	.probe		= dp83tg720_probe,
	.config_aneg	= dp83tg720_config_aneg,
	.read_status	= dp83tg720_read_status,
	.get_features	= genphy_c45_pma_read_ext_abilities,
@@ -355,6 +513,9 @@ static struct phy_driver dp83tg720_driver[] = {
	.get_sqi_max	= dp83tg720_get_sqi_max,
	.cable_test_start = dp83tg720_cable_test_start,
	.cable_test_get_status = dp83tg720_cable_test_get_status,
	.get_link_stats	= dp83tg720_get_link_stats,
	.get_phy_stats	= dp83tg720_get_phy_stats,
	.update_stats	= dp83tg720_update_stats,

	.suspend	= genphy_suspend,
	.resume		= genphy_resume,
+63 −0
Original line number Diff line number Diff line
@@ -615,6 +615,49 @@ int phy_ethtool_get_stats(struct phy_device *phydev,
}
EXPORT_SYMBOL(phy_ethtool_get_stats);

/**
 * __phy_ethtool_get_phy_stats - Retrieve standardized PHY statistics
 * @phydev: Pointer to the PHY device
 * @phy_stats: Pointer to ethtool_eth_phy_stats structure
 * @phydev_stats: Pointer to ethtool_phy_stats structure
 *
 * Fetches PHY statistics using a kernel-defined interface for consistent
 * diagnostics. Unlike phy_ethtool_get_stats(), which allows custom stats,
 * this function enforces a standardized format for better interoperability.
 */
void __phy_ethtool_get_phy_stats(struct phy_device *phydev,
				 struct ethtool_eth_phy_stats *phy_stats,
				 struct ethtool_phy_stats *phydev_stats)
{
	if (!phydev->drv || !phydev->drv->get_phy_stats)
		return;

	mutex_lock(&phydev->lock);
	phydev->drv->get_phy_stats(phydev, phy_stats, phydev_stats);
	mutex_unlock(&phydev->lock);
}

/**
 * __phy_ethtool_get_link_ext_stats - Retrieve extended link statistics for a PHY
 * @phydev: Pointer to the PHY device
 * @link_stats: Pointer to the structure to store extended link statistics
 *
 * Populates the ethtool_link_ext_stats structure with link down event counts
 * and additional driver-specific link statistics, if available.
 */
void __phy_ethtool_get_link_ext_stats(struct phy_device *phydev,
				      struct ethtool_link_ext_stats *link_stats)
{
	link_stats->link_down_events = READ_ONCE(phydev->link_down_events);

	if (!phydev->drv || !phydev->drv->get_link_stats)
		return;

	mutex_lock(&phydev->lock);
	phydev->drv->get_link_stats(phydev, link_stats);
	mutex_unlock(&phydev->lock);
}

/**
 * phy_ethtool_get_plca_cfg - Get PLCA RS configuration
 * @phydev: the phy_device struct
@@ -1398,6 +1441,23 @@ static int phy_enable_interrupts(struct phy_device *phydev)
	return phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
}

/**
 * phy_update_stats - Update PHY device statistics if supported.
 * @phydev: Pointer to the PHY device structure.
 *
 * If the PHY driver provides an update_stats callback, this function
 * invokes it to update the PHY statistics. If not, it returns 0.
 *
 * Return: 0 on success, or a negative error code if the callback fails.
 */
static int phy_update_stats(struct phy_device *phydev)
{
	if (!phydev->drv->update_stats)
		return 0;

	return phydev->drv->update_stats(phydev);
}

/**
 * phy_request_interrupt - request and enable interrupt for a PHY device
 * @phydev: target phy_device struct
@@ -1467,6 +1527,9 @@ static enum phy_state_work _phy_state_machine(struct phy_device *phydev)
	case PHY_RUNNING:
		err = phy_check_link_status(phydev);
		func = &phy_check_link_status;

		if (!err)
			err = phy_update_stats(phydev);
		break;
	case PHY_CABLETEST:
		err = phydev->drv->cable_test_get_status(phydev, &finished);
Loading