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

On Fri, Aug 07, 2026 at 08:39:00AM +0200, Birger Koblitz wrote:
> 
> 
> On 06/08/2026 23:16, Andrew Lunn wrote:
> > > > > +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.
> > > > 
> > > I tried forcing the PHY speed in the past, and it does not work.
> > > 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().
> The error is handled quite poorly:
> $ 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.

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() ?

    Andrew