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

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