Re: [PATCH v3 2/9] vfio: selftests: igb: Use PHY internal loopback on 82576

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message incorrectly references removing and keeping macros and code that do not exist in the baseline or the patch diff.
--

--- Patch [2]: [PATCH v3 2/9] vfio: selftests: igb: Use PHY internal loopback on 82576 ---
commit 2d54b65042a5cbe6702c330df52d94e7c2ff3b57
Author: Alex Williamson <alex.williamson@nvidia.com>

vfio: selftests: igb: Use PHY internal loopback on 82576

This commit replaces igb_phy_setup_autoneg() with igb_setup_loopback() to use
PHY internal loopback on the 82576 device instead of MAC loopback, aligning
with the datasheet requirements for real hardware. It forces the MAC into 1Gb/s
full duplex with link up to ensure the descriptor engine runs. The patch also
maintains compatibility with QEMU's emulated igb behavior without needing
environment-specific code paths.

> The submitted driver also wrote bit 14 (link disable) to PHY register
> 16 (port control).  Datasheet 3.5.6.3.1 describes this as required for
> 10/100Mb/s but explicitly "not a must for 1G", and the kernel ethtool
> selftest omits it.  It is dropped here.  Flagging the removal in case a
> future regression bisects to this commit.
> 
> Remove igb_read_phy() and the PHY status macros it served, which become
> unused: IGB_PHY_STATUS_REG_OFFSET, IGB_PHY_STATUS_AN_COMP.  Keep
> IGB_PHY_CTRL_AN_ENABLE and IGB_PHY_CTRL_AN_RESTART for the QEMU-only
> autoneg kick; add the PHY ctrl loopback and CTRL force-link macros.

[Severity: Medium]
This isn't a bug in the code, but the commit message references removing and
keeping macros and code that do not exist in the baseline or the patch diff.

Looking at the baseline code being modified, igb_phy_setup_autoneg() did not
write to PHY register 16. It also used standard macros like MII_BMCR and
BMSR_ANEGCOMPLETE rather than IGB_PHY_STATUS_REG_OFFSET,
IGB_PHY_STATUS_AN_COMP, or IGB_PHY_CTRL_AN_ENABLE.

Could this part of the commit message be a leftover from an earlier version
of the patch series?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-igb_v3_b4-v3-0-1272650ca614@google.com?part=2