[PATCH v1] net/axgbe: fix PHY auto-negotiation advertisement

[PATCH v1] net/axgbe: fix PHY auto-negotiation advertisement

[Ashok Kumar Natarajan]

Correct the programming of PHY auto-negotiation advertisement registers.

Configure Clause 22 PHY registers according to IEEE 802.3 by advertising
10/100 link capabilities and flow control through MII_ADVERTISE, and
1000BASE-T capabilities through MII_CTRL1000. This avoids advertising
unsupported or unintended features such as unconditional full-duplex
and pause.

Fixes: 7ba7d89890ab ("net/axgbe: support Marvell M88E1512 PHY")
Cc: stable@dpdk.org

Signed-off-by: Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com>
---
 drivers/net/axgbe/axgbe_phy.h      |  5 +++++
 drivers/net/axgbe/axgbe_phy_impl.c | 29 +++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h
index e5568cce5f..12b1d47244 100644
--- a/drivers/net/axgbe/axgbe_phy.h
+++ b/drivers/net/axgbe/axgbe_phy.h
@@ -156,6 +156,11 @@
 #define AXGBE_M88E1512_MODE_RGMII_SGMII		4
 #define AXGBE_M88E1512_MODE_SW_RESET		0x8000
 
+#define AXGBE_PHY_MII_CTRL1000_1000T_HALF   0x0100
+#define AXGBE_PHY_MII_CTRL1000_1000T_FULL   0x0200
+#define AXGBE_PHY_MII_CTRL1000_MS_VALUE     0x0800
+#define AXGBE_PHY_MII_CTRL1000_MS_MANUAL    0x1000
+
 
 /* Control register 1. */
 /* Enable extended speed selection */
diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 369d766884..b340ce02dd 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -303,21 +303,38 @@ static int axgbe_phy_write(struct axgbe_port *pdata, u16 reg, u16 value)
 
 static int axgbe_phy_config_advert(struct axgbe_port *pdata)
 {
+	u32 adv = pdata->phy.advertising;
 	u16 advert;
+	u16 ctrl1000;
 	int ret;
 
-	ret = pdata->phy_if.phy_impl.read(pdata, MII_ADVERTISE, &advert);
+	advert = ADVERTISE_CSMA;
+
+	if (adv & ADVERTISED_10baseT_Full)
+		advert |= ADVERTISE_10FULL;
+
+	if (adv & ADVERTISED_100baseT_Full)
+		advert |= ADVERTISE_100FULL;
+
+	if (adv & ADVERTISED_Pause)
+		advert |= ADVERTISE_PAUSE_CAP;
+
+	if (adv & ADVERTISED_Asym_Pause)
+		advert |= ADVERTISE_PAUSE_ASYM;
+
+	ret = pdata->phy_if.phy_impl.write(pdata, MII_ADVERTISE, advert);
 	if (ret) {
-		PMD_DRV_LOG_LINE(ERR, "Failed to read ADVERTISE register");
+		PMD_DRV_LOG_LINE(ERR, "Failed to write ADVERTISE register");
 		return ret;
 	}
 
-	advert |= ADVERTISE_FULL;
-	advert |= ADVERTISE_PAUSE_CAP;
+	ctrl1000 = 0;
+	if (adv & ADVERTISED_1000baseT_Full)
+		ctrl1000 |= AXGBE_PHY_MII_CTRL1000_1000T_FULL;
 
-	ret = pdata->phy_if.phy_impl.write(pdata, MII_ADVERTISE, advert);
+	ret = pdata->phy_if.phy_impl.write(pdata, MII_CTRL1000, ctrl1000);
 	if (ret) {
-		PMD_DRV_LOG_LINE(ERR, "Failed to write ADVERTISE register");
+		PMD_DRV_LOG_LINE(ERR, "Failed to write MII_CTRL1000 register");
 		return ret;
 	}
 	return 0;
-- 
2.34.1

[PATCH v2] net/axgbe: fix PHY auto-negotiation advertisement

[Ashok Kumar Natarajan]

Correct the programming of PHY auto-negotiation advertisement
registers.

AXGBE supports 10/100/1000BASE-T operation in full-duplex mode
only. Since half-duplex operation (CSMA/CD) is not supported by
the MAC, half-duplex capabilities are intentionally not
advertised. Only supported full-duplex and pause capabilities
are programmed into MII_ADVERTISE and MII_CTRL1000.

This ensures Clause 22 PHY registers are programmed according
to IEEE 802.3 and prevents advertising unsupported or
unintended capabilities.

Fixes: 7ba7d89890ab ("net/axgbe: support Marvell M88E1512 PHY")
Cc: stable@dpdk.org

Signed-off-by: Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com>
---
 drivers/net/axgbe/axgbe_phy.h      |  1 +
 drivers/net/axgbe/axgbe_phy_impl.c | 43 +++++++++++++++++++++++++-----
 2 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h
index e5568cce5f..ab9c692d47 100644
--- a/drivers/net/axgbe/axgbe_phy.h
+++ b/drivers/net/axgbe/axgbe_phy.h
@@ -156,6 +156,7 @@
 #define AXGBE_M88E1512_MODE_RGMII_SGMII		4
 #define AXGBE_M88E1512_MODE_SW_RESET		0x8000
 
+#define AXGBE_PHY_MII_CTRL1000_1000T_FULL	0x0200
 
 /* Control register 1. */
 /* Enable extended speed selection */
diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 369d766884..aa245c1b44 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -303,21 +303,52 @@ static int axgbe_phy_write(struct axgbe_port *pdata, u16 reg, u16 value)
 
 static int axgbe_phy_config_advert(struct axgbe_port *pdata)
 {
+	u32 adv = pdata->phy.advertising;
 	u16 advert;
+	u16 ctrl1000;
 	int ret;
 
-	ret = pdata->phy_if.phy_impl.read(pdata, MII_ADVERTISE, &advert);
+	/*
+	 * AXGBE supports 10BASE-T and 100BASE-TX in full-duplex mode only.
+	 * Half-duplex operation (CSMA/CD) is not supported by the MAC,
+	 * so 10/100 half-duplex capabilities are intentionally not advertised.
+	 * Any ADVERTISED_10baseT_Half or ADVERTISED_100baseT_Half bits
+	 * present in the software advertising mask are therefore ignored.
+	 */
+	advert = ADVERTISE_CSMA;
+
+	if (adv & ADVERTISED_10baseT_Full)
+		advert |= ADVERTISE_10FULL;
+
+	if (adv & ADVERTISED_100baseT_Full)
+		advert |= ADVERTISE_100FULL;
+
+	if (adv & ADVERTISED_Pause)
+		advert |= ADVERTISE_PAUSE_CAP;
+
+	if (adv & ADVERTISED_Asym_Pause)
+		advert |= ADVERTISE_PAUSE_ASYM;
+
+	ret = pdata->phy_if.phy_impl.write(pdata, MII_ADVERTISE, advert);
 	if (ret) {
-		PMD_DRV_LOG_LINE(ERR, "Failed to read ADVERTISE register");
+		PMD_DRV_LOG_LINE(ERR, "Failed to write ADVERTISE register");
 		return ret;
 	}
 
-	advert |= ADVERTISE_FULL;
-	advert |= ADVERTISE_PAUSE_CAP;
+	/*
+	 * AXGBE supports 1000BASE-T in full-duplex mode only.
+	 * Half-duplex operation (CSMA/CD) is not supported by the MAC,
+	 * so 1000BASE-T half-duplex is intentionally not advertised.
+	 * Any ADVERTISED_1000baseT_Half bit present in the software
+	 * advertising mask is therefore ignored here.
+	 */
+	ctrl1000 = 0;
+	if (adv & ADVERTISED_1000baseT_Full)
+		ctrl1000 |= AXGBE_PHY_MII_CTRL1000_1000T_FULL;
 
-	ret = pdata->phy_if.phy_impl.write(pdata, MII_ADVERTISE, advert);
+	ret = pdata->phy_if.phy_impl.write(pdata, MII_CTRL1000, ctrl1000);
 	if (ret) {
-		PMD_DRV_LOG_LINE(ERR, "Failed to write ADVERTISE register");
+		PMD_DRV_LOG_LINE(ERR, "Failed to write MII_CTRL1000 register");
 		return ret;
 	}
 	return 0;
-- 
2.43.0

[PATCH v3] net/axgbe: mask unsupported PHY half-duplex modes

[Ashok Kumar Natarajan]

The AXGBE MAC supports only full-duplex operation at all speeds.
However, the PHY auto-negotiation configuration could advertise
half-duplex modes, including 10BASE-T, 100BASE-TX, and 1000BASE-T,
which are not supported by the MAC.

Update the Clause 22 and Clause 40 PHY advertisement handling to mask
all half-duplex modes while preserving existing PHY, strap, and
vendor-specific configuration using read-modify-write.

To maintain backward compatibility, full-duplex advertisement for
10/100 and 1000BASE-T is preserved by default when no explicit PHY
advertising mask is provided.

This ensures IEEE 802.3 compliant PHY advertisement while avoiding
regressions on platforms relying on PHY default configuration.

Signed-off-by: Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com>
---
 drivers/net/axgbe/axgbe_phy.h      |   3 +
 drivers/net/axgbe/axgbe_phy_impl.c | 102 +++++++++++++++++++++++++++--
 2 files changed, 99 insertions(+), 6 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy.h b/drivers/net/axgbe/axgbe_phy.h
index e5568cce5f..3c00adfebc 100644
--- a/drivers/net/axgbe/axgbe_phy.h
+++ b/drivers/net/axgbe/axgbe_phy.h
@@ -156,6 +156,9 @@
 #define AXGBE_M88E1512_MODE_RGMII_SGMII		4
 #define AXGBE_M88E1512_MODE_SW_RESET		0x8000
 
+/* IEEE 802.3 Clause 40: 1000BASE-T Advertisement Control */
+#define AXGBE_PHY_MII_CTRL1000_1000T_FULL	0x0200
+#define AXGBE_PHY_MII_CTRL1000_1000T_HALF	0x0100
 
 /* Control register 1. */
 /* Enable extended speed selection */
diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index 369d766884..0f0c80f127 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -303,23 +303,113 @@ static int axgbe_phy_write(struct axgbe_port *pdata, u16 reg, u16 value)
 
 static int axgbe_phy_config_advert(struct axgbe_port *pdata)
 {
-	u16 advert;
+	u32 adv = pdata->phy.advertising;
+	u16 advert, orig_advert;
+	u16 ctrl1000, orig_ctrl1000;
 	int ret;
 
+	/*
+	 * Clause 22 (10/100) advertisement configuration.
+	 *
+	 * AXGBE MAC supports only full-duplex operation.
+	 * Half-duplex modes are masked while preserving any
+	 * PHY-specific or reserved bits.
+	 */
 	ret = pdata->phy_if.phy_impl.read(pdata, MII_ADVERTISE, &advert);
 	if (ret) {
-		PMD_DRV_LOG_LINE(ERR, "Failed to read ADVERTISE register");
+		PMD_DRV_LOG_LINE(ERR,
+				"PHY read failed: MII_ADVERTISE");
 		return ret;
 	}
 
-	advert |= ADVERTISE_FULL;
-	advert |= ADVERTISE_PAUSE_CAP;
+	orig_advert = advert;
+
+	/* Always advertise IEEE 802.3 CSMA/CD selector */
+	advert |= ADVERTISE_CSMA;
+
+	/* AXGBE does not support 10/100 half-duplex */
+	advert &= ~(ADVERTISE_10HALF | ADVERTISE_100HALF);
+
+	/*
+	 * Treat adv == 0 as a legacy/default configuration where the driver
+	 * does not impose an explicit policy and preserves the historical
+	 * behavior of advertising all supported full-duplex speeds.
+	 */
+	if (!adv) {
+		advert |= ADVERTISE_10FULL | ADVERTISE_100FULL;
+	} else {
+		if (adv & ADVERTISED_10baseT_Full)
+			advert |= ADVERTISE_10FULL;
+		else
+			advert &= ~ADVERTISE_10FULL;
+
+		if (adv & ADVERTISED_100baseT_Full)
+			advert |= ADVERTISE_100FULL;
+		else
+			advert &= ~ADVERTISE_100FULL;
+
+		if (adv & ADVERTISED_Pause)
+			advert |= ADVERTISE_PAUSE_CAP;
+		else
+			advert &= ~ADVERTISE_PAUSE_CAP;
 
-	ret = pdata->phy_if.phy_impl.write(pdata, MII_ADVERTISE, advert);
+		if (adv & ADVERTISED_Asym_Pause)
+			advert |= ADVERTISE_PAUSE_ASYM;
+		else
+			advert &= ~ADVERTISE_PAUSE_ASYM;
+	}
+
+	if (advert != orig_advert) {
+		ret = pdata->phy_if.phy_impl.write(pdata,
+				MII_ADVERTISE,
+				advert);
+		if (ret) {
+			PMD_DRV_LOG_LINE(ERR,
+					"PHY write failed: MII_ADVERTISE");
+			return ret;
+		}
+	}
+
+	/*
+	 * Clause 40 (1000BASE-T) advertisement configuration.
+	 *
+	 * AXGBE MAC supports only full-duplex operation at 1Gbps.
+	 * Half-duplex advertisement is always cleared.
+	 * Existing PHY or vendor-specific bits are preserved.
+	 */
+	ret = pdata->phy_if.phy_impl.read(pdata, MII_CTRL1000, &ctrl1000);
 	if (ret) {
-		PMD_DRV_LOG_LINE(ERR, "Failed to write ADVERTISE register");
+		PMD_DRV_LOG_LINE(ERR,
+				"PHY read failed: MII_CTRL1000");
 		return ret;
 	}
+
+	orig_ctrl1000 = ctrl1000;
+
+	/* Clear unsupported 1000BASE-T half-duplex */
+	ctrl1000 &= ~AXGBE_PHY_MII_CTRL1000_1000T_HALF;
+
+	/*
+	 * As with Clause 22 advertisement, adv == 0 indicates that no explicit
+	 * advertising policy was requested. In this case, preserve the legacy
+	 * behaviour of advertising 1000BASE-T full-duplex by default.
+	 */
+	if (!adv || (adv & ADVERTISED_1000baseT_Full))
+		ctrl1000 |= AXGBE_PHY_MII_CTRL1000_1000T_FULL;
+	else
+		ctrl1000 &= ~AXGBE_PHY_MII_CTRL1000_1000T_FULL;
+
+	if (ctrl1000 != orig_ctrl1000) {
+		ret = pdata->phy_if.phy_impl.write(pdata,
+				MII_CTRL1000,
+				ctrl1000);
+		if (ret) {
+			PMD_DRV_LOG_LINE(ERR,
+					"PHY write failed: MII_CTRL1000");
+			return ret;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.43.0

Re: [PATCH v3] net/axgbe: mask unsupported PHY half-duplex modes

[Stephen Hemminger]

On Thu, 2 Apr 2026 12:24:49 +0530
Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com> wrote:

> The AXGBE MAC supports only full-duplex operation at all speeds.
> However, the PHY auto-negotiation configuration could advertise
> half-duplex modes, including 10BASE-T, 100BASE-TX, and 1000BASE-T,
> which are not supported by the MAC.
> 
> Update the Clause 22 and Clause 40 PHY advertisement handling to mask
> all half-duplex modes while preserving existing PHY, strap, and
> vendor-specific configuration using read-modify-write.
> 
> To maintain backward compatibility, full-duplex advertisement for
> 10/100 and 1000BASE-T is preserved by default when no explicit PHY
> advertising mask is provided.
> 
> This ensures IEEE 802.3 compliant PHY advertisement while avoiding
> regressions on platforms relying on PHY default configuration.
> 
> Signed-off-by: Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com>
> ---

AI review noticed possible issues with thsi.
Which may be an issue, or false positive.



> The AXGBE MAC supports only full-duplex operation at all speeds.
> However, the PHY auto-negotiation configuration could advertise
> half-duplex modes, including 10BASE-T, 100BASE-TX, and 1000BASE-T,
> which are not supported by the MAC.
> 
> Update the Clause 22 and Clause 40 PHY advertisement handling to mask
> all half-duplex modes while preserving existing PHY, strap, and
> vendor-specific configuration using read-modify-write.

The read-modify-write approach is good, and the Clause 22 / Clause 40
separation is clean.

> +	if (!adv) {
> +		advert |= ADVERTISE_10FULL | ADVERTISE_100FULL;
> +	} else {

The original code unconditionally sets ADVERTISE_PAUSE_CAP:

	advert |= ADVERTISE_FULL;
	advert |= ADVERTISE_PAUSE_CAP;

In the legacy path (adv == 0), the new code advertises 10/100 full-duplex
but drops pause advertisement. This is a behavioral regression for
platforms relying on the default configuration -- exactly the scenario the
commit message says should avoid regressions. The !adv block should also
set ADVERTISE_PAUSE_CAP to match the old behavior.

> +#define AXGBE_PHY_MII_CTRL1000_1000T_FULL	0x0200
> +#define AXGBE_PHY_MII_CTRL1000_1000T_HALF	0x0100

These are standard IEEE 802.3 Clause 40 bits. Since axgbe_phy.h already
mirrors the Linux MII defines for Clause 22, consider adding these as
ADVERTISE_1000FULL / ADVERTISE_1000HALF in the existing MII section for
consistency rather than under the M88E1512-specific block.

Re: [PATCH v3] net/axgbe: mask unsupported PHY half-duplex modes

[Stephen Hemminger]

On Thu, 2 Apr 2026 12:24:49 +0530
Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com> wrote:

> The AXGBE MAC supports only full-duplex operation at all speeds.
> However, the PHY auto-negotiation configuration could advertise
> half-duplex modes, including 10BASE-T, 100BASE-TX, and 1000BASE-T,
> which are not supported by the MAC.
> 
> Update the Clause 22 and Clause 40 PHY advertisement handling to mask
> all half-duplex modes while preserving existing PHY, strap, and
> vendor-specific configuration using read-modify-write.
> 
> To maintain backward compatibility, full-duplex advertisement for
> 10/100 and 1000BASE-T is preserved by default when no explicit PHY
> advertising mask is provided.
> 
> This ensures IEEE 802.3 compliant PHY advertisement while avoiding
> regressions on platforms relying on PHY default configuration.
> 
> Signed-off-by: Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com>
> ---

AI review noticed possible issues with thsi.
Which may be an issue, or false positive.



> The AXGBE MAC supports only full-duplex operation at all speeds.
> However, the PHY auto-negotiation configuration could advertise
> half-duplex modes, including 10BASE-T, 100BASE-TX, and 1000BASE-T,
> which are not supported by the MAC.
> 
> Update the Clause 22 and Clause 40 PHY advertisement handling to mask
> all half-duplex modes while preserving existing PHY, strap, and
> vendor-specific configuration using read-modify-write.

The read-modify-write approach is good, and the Clause 22 / Clause 40
separation is clean.

> +	if (!adv) {
> +		advert |= ADVERTISE_10FULL | ADVERTISE_100FULL;
> +	} else {

The original code unconditionally sets ADVERTISE_PAUSE_CAP:

	advert |= ADVERTISE_FULL;
	advert |= ADVERTISE_PAUSE_CAP;

In the legacy path (adv == 0), the new code advertises 10/100 full-duplex
but drops pause advertisement. This is a behavioral regression for
platforms relying on the default configuration -- exactly the scenario the
commit message says should avoid regressions. The !adv block should also
set ADVERTISE_PAUSE_CAP to match the old behavior.

> +#define AXGBE_PHY_MII_CTRL1000_1000T_FULL	0x0200
> +#define AXGBE_PHY_MII_CTRL1000_1000T_HALF	0x0100

These are standard IEEE 802.3 Clause 40 bits. Since axgbe_phy.h already
mirrors the Linux MII defines for Clause 22, consider adding these as
ADVERTISE_1000FULL / ADVERTISE_1000HALF in the existing MII section for
consistency rather than under the M88E1512-specific block.