Re: [PATCH net-next v10 00/15] ax88179_178a: Add support for AX88179A-based chips

From: Birger Koblitz

Date: Sun Sep 06 2026 - 14:57:08 EST


Hi Jianhui,

On 06/09/2026 12:40 pm, Jianhui Xu wrote:
Hi Birger,

I tested v9 on the same ASIX AX88179B adapter. By the time you read this,
v10 has already been posted. Given the relatively small differences between
v9 and v10, I expect the issue I found in v9 to apply to v10 as well.

First, as you already noticed, `data` is undeclared.

For QEMU runtime testing I also had to apply Chen-Yu Tsai's unrelated
`usb: xhci: Fix HCS_ERST_MAX conversion` patch. Without it, this base kernel
fails to initialize QEMU's xHCI controller before the network driver is
reached.

All three fresh functional starts completed cold DHCP at 1000baseT/Full
without reloading the driver and passed the normal 1000/100/10/1000 Mbit/s
matrix, EEE disable/restore, pause enable/restore, and EEPROM read.

During repeated speed transitions, I observed one intermittent carrier-loss
failure. Of 19 normally initiated restores from 100/full to the default
1000/full advertisement, 18 passed and one failed to regain carrier. After
that failure, ethtool reported unknown speed and no link, and further
advertisement changes did not recover it. Dmesg showed the preceding
100-Mbit Link Up followed by Link Down, with no subsequent Link Up before
the device was reattached.

I could not reproduce the failure in two later fresh starts or in a further
90 unmodified-v9 stress cycles. Counting the normal matrix and stress
points, all 22 tested 100/full points passed, so I also did not reproduce
the earlier 100-Mbit carrier-without-RX failure.

I tried to reproduce the issue myself. I am running the v10 USB and PHY driver modules directly on a PC with otherwise Kernel
Linux version 7.1.8+deb13-amd64

I am using the following script to change between 100MBit and 1GBit
1000 times with an AX88179A 1GBit dongle, each time testing a ping and printing out the negotiated speed:
!/bin/bash
i=0
speed=100
while [ $i -lt 1000 ]
do
((i++))
echo "Run $i"
success="BAD"
success=$(ping -c 1 -q 192.168.10.247 >1 2>&1 && echo "OK")
echo "Success: $success"

if [ "$success" != "OK" ]; then
echo "Failed: $success $i"
fi

ethtool -s enx9c69d380ffff autoneg on speed $speed
if [ "$speed" != "100" ]; then
speed=100
else
speed=1000
fi
sleep 6
echo `ethtool enx9c69d380ffff | grep Speed:`
done

This runs through without issues 1000 times:
[...]
Success: OK
Speed: 1000Mb/s
Run 999
Success: OK
Speed: 100Mb/s
Run 1000
Success: OK
Speed: 1000Mb/s

ethtool changes the speed, then reliably reads the correct speed while ping works and the device continues to respond normally through the entire test. All of this 1000 times.

So, either what you observed is extremely rare, or it could also be influenced by the different test setup, including the fact my test is on bare metal, but also by the link partner (an RTL8372-switch in my case).

The other point is that in principle what we are testing is not something users do regularly: you plug the dongle in, it autonegotiates, and that's it. And if you change the speed, there is a 50% chance you will do it from the other side of the link, which seems to be much less problematic as I noticed previously when we had the race condition due to PHY polling, because the dongle does not try to optimize the link: it accepts what is being offered.

I did not test your code, but I am wondering whether it is not simply enough to do:
+ if (link)
+ phylink_mac_interrupt(data->phylink);
without reading the bit whether the MAC link is up. Speed changes come
with a link-down, so I would think this is enough to do it like that. Or maybe I am missing something?

Given that we do not really know whether there is actually an issue, what it is and whether it is really relevant, otherwise your tests all succeed, and also because I have a feeling that Andrew may not like it if we try to be smarter than phylink, I would suggest we leave it like this for the moment. I would really like to see this being finally reviewed to the point where it can be merged.

Birger