Am 10.03.25 um 19:06 schrieb Alan Stern:
On Mon, Mar 10, 2025 at 05:59:31PM +0100, Fiona Klute wrote:
If a new reset event appears before the previous one has been processed, the device can get stuck into a reset loop. This happens rarely, but blocks the device when it does, and floods the log with messages like the following:
lan78xx 2-3:1.0 enp1s0u3: kevent 4 may have been dropped
The only bit that the driver pays attention to in the interrupt data is "link was reset". If there's a flapping status bit in that endpoint data (such as if PHY negotiation needs a few tries to get a stable link), polling at a slower rate allows the state to settle.
This is a simplified version of a patch that's been in the Raspberry Pi downstream kernel since their 4.14 branch, see also: https://github.com/raspberrypi/linux/issues/2447
Signed-off-by: Fiona Klute fiona.klute@gmx.de Cc: kernel-list@raspberrypi.com Cc: stable@vger.kernel.org
For the stable crew: I've *tested* the patch with 6.12.7 and 6.13.5 on a Revolution Pi Connect 4 (Raspberry Pi CM4 based device with built-in LAN7800 as second ethernet port), according to the linked issue for the RPi downstream kernel the problem should be present in all maintained longterm kernel versions, too (based on how long they've carried a patch).
drivers/net/usb/lan78xx.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index a91bf9c7e31d..7bf01a31a932 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -173,6 +173,12 @@ #define INT_EP_GPIO_1 (1) #define INT_EP_GPIO_0 (0)
+/* highspeed device, so polling interval is in microframes (eight per
- millisecond)
- */
+#define INT_URB_MICROFRAMES_PER_MS 8 +#define MIN_INT_URB_INTERVAL_MS 8
- static const char lan78xx_gstrings[][ETH_GSTRING_LEN] = { "RX FCS Errors", "RX Alignment Errors",
@@ -4527,7 +4533,11 @@ static int lan78xx_probe(struct usb_interface *intf, if (ret < 0) goto out4;
- period = ep_intr->desc.bInterval;
- period = max(ep_intr->desc.bInterval,
MIN_INT_URB_INTERVAL_MS * INT_URB_MICROFRAMES_PER_MS);This calculation is completely wrong. For high-speed interrupt endpoints, the bInterval value is encoded using a logarithmic scheme. The actual interval in microframes is given by 2^(bInterval - 1) (see Table 9-13 in the USB 2.0 spec). Furthermore, when the value is passed to usb_fill_int_urb(), the interval argument must be encoded in the same way (see the kerneldoc for usb_fill_int_urb() in include/linux/usb.h).
The encoded value corresponding to 8 ms is 7, not 64, since 8 ms = 64 uframes and 64 = 2^(7-1).
Thanks, I'll have to fix that if I send a revision. Though after Andrew Lunn's response and seeing the bug again today despite the patch after a few hundred good boots I suspect a different fix is needed.
- dev_info(&intf->dev,
"interrupt urb period set to %d, bInterval is %d\n",period, ep_intr->desc.bInterval);I doubt that this dev_info() will be very helpful to anyone (in addition to being wrong since the value in "period" is not the actual period).
I figured it'd be useful to note that the interval has been modified, especially for possible debugging.
Best regards, Fiona