| Message ID | m34j43gwto.fsf@t19.piap.pl (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | usbnet_link_change() fails to call netif_carrier_on() | expand |
On Tue, Nov 19, 2024 at 02:46:59PM +0100, Krzysztof Hałasa wrote: > Hi, > > ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come > up ("carrier off"), despite the built-in 100BASE-FX PHY positive link > indication. > > The problem appears to be in usbnet.c framework: > > void usbnet_link_change(struct usbnet *dev, bool link, bool need_reset) > { > /* update link after link is reseted */ > if (link && !need_reset) > netif_carrier_on(dev->net); > else > netif_carrier_off(dev->net); > > if (need_reset && link) > usbnet_defer_kevent(dev, EVENT_LINK_RESET); > else > usbnet_defer_kevent(dev, EVENT_LINK_CHANGE); > } static int ax88772_phylink_setup(struct usbnet *dev) { struct asix_common_private *priv = dev->driver_priv; phy_interface_t phy_if_mode; struct phylink *phylink; priv->phylink_config.dev = &dev->net->dev; priv->phylink_config.type = PHYLINK_NETDEV; priv->phylink_config.mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | MAC_10 | MAC_100; etc. This device is using phylink to manage the PHY. phylink will than manage the carrier. It assumes it is solely responsible for the carrier. So i think your fix is wrong. You probably should be removing all code in this driver which touches the carrier. Andrew
--- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1978,16 +1978,18 @@ EXPORT_SYMBOL(usbnet_manage_power); void usbnet_link_change(struct usbnet *dev, bool link, bool need_reset) { - /* update link after link is reseted */ - if (link && !need_reset) + if (link) netif_carrier_on(dev->net); else netif_carrier_off(dev->net); - if (need_reset && link) - usbnet_defer_kevent(dev, EVENT_LINK_RESET); - else - usbnet_defer_kevent(dev, EVENT_LINK_CHANGE); + if (need_reset) { + /* update link after link is reset */ + if (link) + usbnet_defer_kevent(dev, EVENT_LINK_RESET); + else + usbnet_defer_kevent(dev, EVENT_LINK_CHANGE); + } } EXPORT_SYMBOL(usbnet_link_change);
Hi, ASIX AX88772B based USB 10/100 Ethernet adapter doesn't come up ("carrier off"), despite the built-in 100BASE-FX PHY positive link indication. The problem appears to be in usbnet.c framework: void usbnet_link_change(struct usbnet *dev, bool link, bool need_reset) { /* update link after link is reseted */ if (link && !need_reset) netif_carrier_on(dev->net); else netif_carrier_off(dev->net); if (need_reset && link) usbnet_defer_kevent(dev, EVENT_LINK_RESET); else usbnet_defer_kevent(dev, EVENT_LINK_CHANGE); } I think the author's idea was a bit different than what the code really ended doing. Especially when called with link = 1 and need_reset = 1. It seems it may have already caused problems - possible workarounds: - commit 7be4cb7189f7 ("net: usb: ax88179_178a: improve reset check") - commit ecf848eb934b ("net: usb: ax88179_178a: fix link status when link is set to down/up") Can't check those due to -ENOHW but ax88179_link_reset() adds a netif_carrier_on() call on link ups. Not sure about the "reset" name, though (and the comment in usbnet_link_change()). It seems it's when the link goes up. Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl> Fixes: ac64995da872 ("usbnet: introduce usbnet_link_change API") Fixes: 4b49f58fff00 ("usbnet: handle link change") The code has been introduced in 2013, by 4b49f58fff00 and a bunch of related commits. Perhaps it visibly affects only AXIS and dm9601 adapters, though.