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

[Stephen Hemminger <stephen@networkplumber.org>]

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.