Re: [PATCH net-next v12 06/15] ax88179_178a: Add HW support for AX179A-based chips

[Birger Koblitz <mail@birger-koblitz.de>]

On 17/09/2026 11:24 pm, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 27 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 4 · Medium: 19 · Low: 4
> 
> - [High] ax88179a_bind() advertises NETIF_F_HW_VLAN_CTAG_FILTER in
>    dev->features/hw_features while ax88179a_netdev_ops provides neither…
The relevant functions are added in a later commit.

> - [High] The three new device IDs inherit the single usb_driver
>    .suspend/.resume/.reset_resume of the legacy AX88179 code…
A later part of the patch series adds dedicated suspend/resume functions 
for these chips.

> - [High] ax88179a_rx_fixup() dereferences device-controlled metadata with
>    plain native-endian u64 pointer loads: `rx_hdr = *(u64…
Will be fixed.

> - [High] ax88179a_rx_fixup() enforces only an upper packet-length bound
>    (`pkt_len > dev->hard_mtu + AX179A_RX_HW_PAD`) and no minimum, so a…
A check for pkt_len < 2 + ETH_HLEN will be added in the next version.

> - [Medium] ax88179a_bind() puts
>    NETIF_F_IP_CSUM/NETIF_F_IPV6_CSUM/NETIF_F_RXCSUM/VLAN offloads into…
A later patch in the series adds the relevant functions.

> - [Medium] ax88179a_bind() sets
>    ax179_data->eeprom_read_cmd/eeprom_write_cmd/eeprom_block/eeprom_wen…
A later patch in the series adds the relevant functions.

> - [Medium] ax88179a_ethtool_ops installs .nway_reset = usbnet_nway_reset,
>    but ax88179a_bind() never fills in dev->mii (only…
Reference to usbnet_nway_reset will be replaced by phy_ethtool_nway_reset.

> - [Medium] ax88179a_stop() prepares `reg8 = 0` to power the Ethernet PHY
>    down but then calls ax88179_read_cmd(dev, AX88179A_PHY_POWER, 0, 0, 1,…
Was typo: ax88179_write_cmd is correct function call. Will fix.

> - [Medium] ax88179a_bulkin_config() starts with `int index = 0` and the
>    ETHER_LINK_1000 / ETHER_LINK_100 cases only assign index when link_sts…
The restriction to ax179_data->chip_version == AX_VERSION_AX88279 is
dropped and for all chip versions index = BULK_IN_SPEED_FS is now set 
when the USB speed is FS.

> - [Medium] ax88179a_ethtool_ops mixes abstraction layers: pause
>    parameters and SIOCxMIIREG go through phylink…
Will use phylink versions of calls for get/set_link_ksettings.

> - [Medium] In ax88179a_reset() the WoL capability probe passes `&tmp` - a
>    `u8 **` - as the destination of a 1-byte control read, while `tmp` is…
Will fix latent bug: &tmp -> tmp.

> - [Medium] ax88179_mdiobus_read() ignores the return value of
>    ax88179_read_cmd() and returns the u16 `res`, which ax88179_read_cmd()…
Will add check for ax88179_read_cmd() return value.

> - [Medium] At this commit ax88179a_netdev_ops omits .ndo_set_rx_mode
>    although the shared library provides ax88179_set_multicast() (which…
Multicast added in a later patch of the series.

> - [Medium] ax88179a_tx_fixup() writes into the SKB tailroom
>    (skb_put_zero()) and headroom (skb_push() + put_unaligned_le64())…
Returning NULL to usbnet when the memory allocation fails is intentional.

> - [Medium] Nothing bridges phylink's carrier-up transition to usbnet's
>    RX-URB submission.
Removed usbnet_link_change(dev, 0, 0); at the end of ax88179a_reset().

> - [Medium] ax88179a_reset() calls phylink_start() but usbnet_open() has
>    failure paths after info->reset() - info->check_connect() and…
Duplicate of above.

> - [Medium] All three new driver_infos set FLAG_AVOID_UNLINK_URBS, so
>    usbnet_stop() skips usbnet_terminate_urbs() and RX URBs stay submitted…
Appears to be false positive and not an issue in practice.

> - [Medium] ax88179a_stop() tears the MAC down (read-modify-write clearing
>    AX_MEDIUM_RECEIVE_EN, then AX_RX_CTL = 0) before calling…
Will move phylink_stop before the receiver is disabled.

> - [Medium] No lock serializes MAC register access and the shared
>    ax179_data fields between the phylink resolve worker…
Will use RTNL to protect AX_MEDIUM_STATUS_MODE register, use
WRITE_ONCE/READ_ONCE for rxctl.

> - [Medium] ax88179a_reset() calls phylink_start() and then
>    usbnet_link_change(dev, 0, 0), giving the netdev two independent…
Duplicate of above, fixed by removing usbnet_link_change(dev, 0, 0).

> - [Medium] In ax88179a_mac_link_up() the SPEED_2500 branch (reachable
>    only on the AX88279, the only chip advertising MAC_2500FD) programs…
Changed to clearing AX_XGMII_EN for all speeds != SPEED_2500 independent
of chip-id.

> - [Medium] In the SPEED_2500 branch of ax88179a_mac_link_up(), the two-
>    byte write to AX88179A_MAC_RX_DATA_CDC_CNT passes the byte array…
Fixed by using 2 1-byte writes.

> - [Medium] net/usb: Silent packet drop on TX memory allocation failure
Intentional.

> - [Low] The commit message tells users to give 'the ax88179_178a module'
>    precedence over CDC-NCM, but the series builds ax88179.ko…
This is a user-space issue prioritizing modules.

> - [Low] ax88179a_tx_fixup() prepends an 8-byte TX descriptor and appends
>    up to 16 bytes of padding but calls usbnet_set_skb_tx_stats(skb, 1,…
Intentional.

> - [Low] ax88179a_mac_link_up() declares `u8 link_sts` without
>    initialization, calls ax88179_read_cmd(dev, AX_ACCESS_MAC,…
Fixed by initializing link_status to 0.

> - [Low] ax88179a_bind() powers the PHY (write AX_PHY_POWER to vendor
>    command AX88179A_PHY_POWER, then msleep(250)) before the fallible…
> 
Intentional: Better not to write further data into a chip that is not 
what is expected.