From: "Krzysztof Hałasa" <khalasa@piap.pl>
To: netdev <netdev@vger.kernel.org>
Cc: Oliver Neukum <oneukum@suse.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Jose Ignacio Tornos Martinez <jtornosm@redhat.com>,
Ming Lei <ming.lei@canonical.com>
Subject: [PATCH] usbnet_link_change() fails to call netif_carrier_on()
Date: Tue, 19 Nov 2024 14:46:59 +0100 [thread overview]
Message-ID: <m34j43gwto.fsf@t19.piap.pl> (raw)
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")
--- 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);
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.
--
Krzysztof "Chris" Hałasa
Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa
next reply other threads:[~2024-11-19 13:55 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-19 13:46 Krzysztof Hałasa [this message]
2024-11-19 16:20 ` [PATCH] usbnet_link_change() fails to call netif_carrier_on() Andrew Lunn
2024-11-21 6:51 ` Krzysztof Hałasa
2024-11-21 14:25 ` Andrew Lunn
2024-11-21 13:22 ` Krzysztof Hałasa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m34j43gwto.fsf@t19.piap.pl \
--to=khalasa@piap.pl \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jtornosm@redhat.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=ming.lei@canonical.com \
--cc=netdev@vger.kernel.org \
--cc=oneukum@suse.com \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.