Re: [PATCH net-next v12 15/15] ax88796b: Add support for AX88772D, AX88179A and AX88279

[Birger Koblitz]

On 17/09/2026 11:25 pm, netdev-bot+sashiko@xxxxxxxxxx wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 6 · Low: 0
>
> - [Medium] asix_ax88279_config_aneg()
>    (drivers/net/phy/ax88796b.c:146-166) programs the vendor 2.5G…
This is tested and works as written and not the other way round. The
firmware of the device seems to initiate autonegotiation on itself when
link-modes are changed in the device (not by the other end of the link)
in order to optimize the link. This is the reason for needing to rely on
link mode changes by "interrupts" from the MAC layer, instead of reading
inaccurate link mode registers.

> - [Medium] asix_ax88279_read_status()
>    (drivers/net/phy/ax88796b.c:108-121) does not check the return value…
Added checking of return value and returning this as an error code.

> - [Medium] asix_ax88279_read_status() (drivers/net/phy/ax88796b.c:99-144)
>    has no `if (!phydev->link) return 0;` guard.
Will add this guard. Indeed it fixes reporting incorrect Speed/Duplex
  values, instead of Unknown/Unknown.

> - [Medium] asix_ax88279_read_status() hard-codes phydev->duplex =
>    DUPLEX_FULL with the comment 'Only supports full duplex'…
All erroneously reported modes are removed, this is successfully tested
with real-world hardware. Removing further hypothetically reported modes
is not necessary. If such modes were hypothetically reported, they may
actually be valid modes, anyway.

> - [Medium] In asix_ax88279_read_status() the link-partner 2.5G capability
>    bit is only ever assigned inside `if (val >= 0 && val &…
Added clearing of ETHTOOL_LINK_MODE_2500baseT_Full_BIT in lp_advertising
before modifying the bit.

> - [Medium] net: phy: ax88796b: incomplete clearing of 1000baseT
>    capabilities for AX88772D
You stated: Only ETHTOOL_LINK_MODE_1000baseT_Full_BIT is put into mask 
and removed
  from phydev->supported here, so if the hardware also sets
  ESTATUS_1000_THALF or ESTATUS_1000_XFULL those bits survive in
  phydev->supported.

These other bits are not actually reported by actual PHYs, which are
anyway just something accessed through the controller firmware. When
hypothesising that they may report e.g. 1000X fiber modes one would also
need to talk about hypothetically reported 10GBit modes. Where would 
that end?
This was tested on various hardware.
The clearing will be shortened to just:
linkmode_clear_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT, phydev->supported)
as for the ax88279-case.