Re: [PATCH v4 2/3] net/axgbe: add support for marvell m88e1512 PHY

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 27 Feb 2026 14:14:29 +0530
Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com> wrote:

> @@ -2556,6 +2922,21 @@ static int axgbe_phy_init(struct axgbe_port *pdata)
>  	}
>  
>  	phy_data->phy_cdr_delay = AXGBE_CDR_DELAY_INIT;
> +
> +	ret = axgbe_get_phy_id(pdata);
> +	if (ret) {
> +		PMD_DRV_LOG_LINE(ERR, "failed to get PHY id");
> +		return ret;
> +	}
> +
> +	PMD_DRV_LOG_LINE(DEBUG, "PHY ID = 0x%x", phy_data->phy_id);
> +
> +	if (phy_data->phy_id == M88E1512_E_PHY_ID) {
> +		ret = axgbe_m88e1512_init(pdata);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }

AI was confused and thinks that there may be an issue here:


Patch 2/3: net/axgbe: add support for marvell m88e1512 PHY
Error — axgbe_get_phy_id() called unconditionally in axgbe_phy_init() (potential regression)
This is the most significant finding in the series. axgbe_get_phy_id() is inserted at the end of axgbe_phy_init() without any guard on port mode:


c
 	phy_data->phy_cdr_delay = AXGBE_CDR_DELAY_INIT;
+
+	ret = axgbe_get_phy_id(pdata);
+	if (ret) {
+		PMD_DRV_LOG_LINE(ERR, "failed to get PHY id");
+		return ret;
+	}
axgbe_phy_init() is called for all port modes — SFP, backplane, 10GBASE-T, etc. For port modes that have no external PHY, axgbe_phy_read() will attempt a Clause 22 MDIO read on phy_data->mdio_addr and likely timeout (-ETIMEDOUT), causing the entire PHY init to fail. This is a regression for devices that previously worked.

The M88E1512-specific init block below already checks phy_data->phy_id, but the axgbe_get_phy_id() call itself must not fail for non-1000BASE-T ports.

Fix: Gate the PHY ID read (and subsequent M88E1512 init) on the port mode:


c
	if (phy_data->port_mode == AXGBE_PORT_MODE_1000BASE_T) {
		ret = axgbe_get_phy_id(pdata);
		if (ret) {
			PMD_DRV_LOG_LINE(ERR, "failed to get PHY id");
			return ret;
		}

		if (phy_data->phy_id == M88E1512_E_PHY_ID) {
			ret = axgbe_m88e1512_init(pdata);
			if (ret)
				return ret;
		}
	}
This is consistent with how axgbe_phy_link_status() in the same patch correctly guards the external PHY check with if (phy_data->port_mode == AXGBE_PORT_MODE_1000BASE_T).

Confidence: High (~90%). The MDIO read will go through hardware I/O regardless of whether a PHY exists at the address.