On Wed, 25 Feb 2026 18:14:54 +0530 Ashok Kumar Natarajan <[email protected]> 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 <[email protected]> > --- 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: [email protected] so it gets backported to stable branches. Item 1 is the most critical — it inverts link status reporting for the M88E1512.
