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

On 07/08/2026 15:14, Andrew Lunn wrote:
> 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.
I'll fix that in v7. I would like to give Jianhui time to test v6, first,
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.

> 
> 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'll work on this right away.

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