Re: [PATCH v2 1/3] net/axgbe: add external PHY read/write functions

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 25 Feb 2026 18:14:54 +0530
Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com> wrote:

> Introduce helper functions to perform external PHY register read and
> write operations. These helpers currently support only IEEE Clause 22
> PHY access, providing a simple and consistent API for accessing
> standard 16‑bit MII registers on external PHY devices.
> 
> Signed-off-by: Ashok Kumar Natarajan <ashokkumar.natarajan@amd.com>
> ---

I didn't see anything here, but AI spotted some things.
Please revise and resubmit.

Thanks for the series. A few findings, mostly in patch 2:
Patch 2 (M88E1512 support):

1. axgbe_phy_link_status() always returns 0 (link up) even when the
M88E1512 reports link down or the read fails. The goto out path leads
to return 0, which means "link up" in this function's contract. The
link-down and error paths need to return a non-zero value. 

2. Integer shift UB in axgbe_get_phy_id(): (u32)(phy_id_1 << 16) evaluates the
shift at int width before widening. If bit 15 of phy_id_1 is set,
shifting into the sign bit is undefined behavior. Move the cast before
the shift: (u32)phy_id_1 << 16. 

3. axgbe_get_ext_phy_link_status() doesn't
set *linkup = false on the BMCR_ANRESTART early-return path. Works
today because the caller pre-initializes the variable, but the function
should be self-contained.

4. The new out label unconditionally clears
rx_adapt_done on the M88E1512 link-down path — please verify this is
intentional and doesn't interfere with the internal SerDes receiver
adaptation state machine.

Patch 3 (100Mbps):

5. The SPEED_1000 → SPEED_100 change in axgbe_sgmii_100_mode() is a bug fix in existing code — the 100M mode function was incorrectly configuring the MAC for 1G. This should have its own Fixes: tag and Cc: stable@dpdk.org so it gets backported to stable branches.

Item 1 is the most critical — it inverts link status reporting for the M88E1512.