Re: [PATCH net-next v5 13/13] ax88796b: Add support for AX88772D, AX88179A and AX88279
From: Birger Koblitz
Date: Fri Aug 07 2026 - 14:06:02 EST
On 07/08/2026 15:14, Andrew Lunn wrote:
On Fri, Aug 07, 2026 at 08:39:00AM +0200, Birger Koblitz wrote:I'll fix that in v7. I would like to give Jianhui time to test v6, first,
On 06/08/2026 23:16, Andrew Lunn wrote:
The error is handled quite poorly:I tried forcing the PHY speed in the past, and it does not work.+static int asix_ax88279_config_aneg(struct phy_device *phydev)
+{
+ bool adv_2500;
+ int ret;
+
+ if (phydev->autoneg == AUTONEG_DISABLE) {
+ phydev_warn(phydev, "Disabling autoneg is not supported\n");
+ return -EINVAL;
+ }
Why is that? Now that phylink is driving the PHY, and reporting
speeds, duplex etc, you should have everything you need to program the
MAC when the link mode is forced.
This is a limitation which is also acknowledged for the suspected underlying PHY
hardware Airoha EN8811H, see the head of air_en8811h.c
How well does this unwind on error? Is phydev->autoneg put back to the
old state to indicate autoneg is used?
This is one of the areas where phylib/phylink is missing a bit of
support infrastructure. We have a bit indicating the PHY support
autoneg, ETHTOOL_LINK_MODE_Autoneg_BIT, but there is no bit
ETHTOOL_LINK_MODE_Force_BIT, indicating the PHY supports forced
mode. So the core cannot enforce this and reject it in
ksettings_set().
$ sudo ethtool -s enx6c6e07090000 autoneg off speed 1000
$ ip a
shows link is down
Kernel log:
[848763.836126] Asix Electronics AX88279 usb-004:045:03: Disabling autoneg is not supported
[848763.836134] ------------[ cut here ]------------
[848763.836137] _phy_start_aneg+0x0/0xa0: returned: -22
[848763.836142] WARNING: drivers/net/phy/phy.c:1352 at _phy_state_machine+0x100/0x310, CPU#8: kworker/8:1/534588
[848763.836146] Modules linked in: ax88179_178a cdc_mbim cdc_ncm ax88179(OE)
ax88796b(OE) cdc_ether phylink(OE) r8152 nfnetlink_queue nfnetlink_log
That is ugly.
Ideally, we want phylib/phylink to report EOPNOTSUPP in
ksetting_set. However, as i said, we currently don't have a mechanism
for the PHY to report forced link does not work.
So:
Please change EINVAL to EOPNOTSUPP.
tough. There were definitely bugs regarding suspend/resume and the MAC configuration
introduced in v4/v5, that I would like to be sure have been fixed.
I'll work on this right away.
We accept your change as is.
But if you have time/energy, it would be nice to have some follow up
patches in a new series which fixes this correctly, allow a PHY to
indicate it does not support forced. The question is, how to actually
do that. The BMSR has a bit `Auto-Negotiation Ability`. But there is
no 'Forced Ability' indication. It is simply assumed all PHYs can do
forced. All the bits we have in phydev->supported, are positive, the
PHY can do something. There are no negative, the PHY cannot do
something. So i think it would be wrong to add a negative.
What we do have is struct phy_driver flags, and currently
#define PHY_IS_INTERNAL 0x00000001
#define PHY_RST_AFTER_CLK_EN 0x00000002
#define PHY_POLL_CABLE_TEST 0x00000004
#define PHY_ALWAYS_CALL_SUSPEND 0x00000008
#define MDIO_DEVICE_IS_PHY 0x80000000
So maybe add
#define PHY_BROKEN_FORCED 0x00000010
and look for that in ksettings_set() ?
I also would like to add another feature to phylink separately. All the USB
Ethernet controllers have a feature where the controller sends a USB Interrupt
URB to the host when the link state changes. It would be great if instead
of polling, this could be used to trigger a read_status() of the PHY instead
of polling it constantly over USB. This would work like the equivalent of the
PHY link state change interrupts. Basically, the host driver would, upon
receiving a link change interrupt URB trigger a similar code path in phylink
as a link change interrupt by a PHY triggers. Ideally, this could be something
that usbnet would help drivers to set up. Would that make sense?
Birger