* [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
@ 2026-09-01 3:49 Julian Oes
2026-09-01 4:33 ` Greg Kroah-Hartman
2026-09-01 8:28 ` Oliver Neukum
0 siblings, 2 replies; 8+ messages in thread
From: Julian Oes @ 2026-09-01 3:49 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Julian Oes
A USB serial port can go permanently silent when its bulk-in endpoint is
halted: the read URBs complete with -EPIPE, which the generic read
callback has always treated as fatal, and no further data arrives until
user space closes and reopens the tty.
Recovery from a halted bulk endpoint is CLEAR_FEATURE(ENDPOINT_HALT)
followed by resubmission of the reads. That needs a control transfer, so
defer it to a work item as cdc-acm already does.
Use a dedicated work item rather than the existing per-port work. The
latter is scheduled from every write completion and must not be
cancelled on close, as the line discipline depends on it. Stall
recovery resubmits the read URBs and must therefore be cancelled
wherever the reads are stopped, that is, on close, suspend and
disconnect.
Schedule the recovery with a short cooldown. The stall can persist for a
few milliseconds, during which the resubmitted URBs stall again as soon
as they are queued; without a delay the recovery becomes a busy loop of
control transfers. Rate-limit the stall message for the same reason, as
a stall is now a recurring condition rather than a terminal one.
I ran into this with an FTDI FT231X behind a hub that halts the
adapter's bulk-in endpoint whenever an unrelated CDC ACM device on a
sibling port is disconnected. With this change the FTDI port keeps
receiving data across such an event.
Signed-off-by: Julian Oes <julian@oes.ch>
Assisted-by: LLM
---
v1: https://lore.kernel.org/linux-usb/20260513023728.55557-1-julian@oes.ch/
(no replies)
Changes since v1:
- Use a dedicated delayed work instead of port->work. port->work is
scheduled from every write completion and must not be cancelled on
close, which stall recovery has to be.
- Cancel the recovery in close, suspend and disconnect. v1 cancelled it
only in disconnect, so a stall reported just before close could
resubmit the read URBs on a closed port.
- Add a 10 ms cooldown and rate-limit the stall message. Retrying
immediately, as cdc-acm does, turned recovery into a busy loop here:
six attempts in six milliseconds, each clearing the halt and
resubmitting only to stall again, until the fault cleared on its own.
- Bail out if usb_clear_halt() fails rather than resubmitting anyway,
and skip the reads if the port is throttled.
- Drop the Fixes tag. fc11efe2800f only made the existing -EPIPE
handling explicit; the preceding "if (urb->status) return;" treated a
stall as fatal too, so this is a missing feature, not a regression.
- Drop my redundant Tested-by, add Assisted-by (see below).
- Rebase onto v7.3-rc1.
Testing: the trigger is racy, about one unplug in ten, and only with both
devices on the same internal hub controller and the FTDI port open with
data flowing. With the patch the halt is cleared and the reads resume; I
confirmed data keeps arriving by holding the port open with screen and
watching the arriving bytes across the event, where before it stopped dead.
Not tested: the close-during-recovery and suspend-with-recovery-pending
paths.
An LLM (Claude Code) assisted throughout: narrowing the problem down from
usbmon traces, drafting the patch, and writing this changelog.
Hardware: SiK telemetry radio (FTDI FT231X, 0403:6015) and a PX4 FMU v6C
(3185:0038) on a j5create JUH377 hub, internally two chained Genesys
Logic GL3523 controllers. The hub is what halts the endpoint; the adapter
itself is innocent. I can capture usbmon traces if that would help.
drivers/usb/serial/generic.c | 87 +++++++++++++++++++++++++++++++--
drivers/usb/serial/usb-serial.c | 12 ++++-
include/linux/usb/serial.h | 4 ++
3 files changed, 98 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 6eaf74930aa3..be3c29defb01 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -114,6 +114,22 @@ int usb_serial_generic_open(struct tty_struct *tty, struct usb_serial_port *port
}
EXPORT_SYMBOL_GPL(usb_serial_generic_open);
+/*
+ * Cooldown between attempts to recover a stalled bulk-in endpoint. A stall
+ * caused by a transient fault can persist for a few milliseconds, during which
+ * the resubmitted URBs stall again immediately. Back off a little so that a
+ * persistently halted endpoint is not hammered with control transfers.
+ */
+#define USB_SERIAL_STALL_COOLDOWN msecs_to_jiffies(10)
+
+static void usb_serial_generic_kill_read_urbs(struct usb_serial_port *port)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
+ usb_kill_urb(port->read_urbs[i]);
+}
+
void usb_serial_generic_close(struct usb_serial_port *port)
{
unsigned long flags;
@@ -128,8 +144,16 @@ void usb_serial_generic_close(struct usb_serial_port *port)
spin_unlock_irqrestore(&port->lock, flags);
}
if (port->bulk_in_size) {
- for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
- usb_kill_urb(port->read_urbs[i]);
+ usb_serial_generic_kill_read_urbs(port);
+ /*
+ * The read URBs are dead now so no further stall can be
+ * reported, but stall recovery may already be running and may
+ * have resubmitted them. Wait for it to finish before killing
+ * the URBs for good.
+ */
+ cancel_delayed_work_sync(&port->stall_work);
+ usb_serial_generic_kill_read_urbs(port);
+ clear_bit(USB_SERIAL_RX_STALLED, &port->flags);
}
}
EXPORT_SYMBOL_GPL(usb_serial_generic_close);
@@ -370,6 +394,7 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
struct usb_serial_port *port = urb->context;
unsigned char *data = urb->transfer_buffer;
bool stopped = false;
+ bool stalled = false;
int status = urb->status;
int i;
@@ -394,9 +419,10 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
stopped = true;
break;
case -EPIPE:
- dev_err(&port->dev, "%s - urb stopped: %d\n",
+ dev_err_ratelimited(&port->dev, "%s - urb stalled: %d\n",
__func__, status);
- stopped = true;
+ set_bit(USB_SERIAL_RX_STALLED, &port->flags);
+ stalled = true;
break;
default:
dev_dbg(&port->dev, "%s - nonzero urb status: %d\n",
@@ -419,6 +445,16 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
*/
smp_mb__after_atomic();
+ /*
+ * Defer stall recovery to a work item as clearing the halt condition
+ * requires a control transfer, which cannot be issued from here.
+ */
+ if (stalled) {
+ schedule_delayed_work(&port->stall_work,
+ USB_SERIAL_STALL_COOLDOWN);
+ return;
+ }
+
if (stopped)
return;
@@ -429,6 +465,49 @@ void usb_serial_generic_read_bulk_callback(struct urb *urb)
}
EXPORT_SYMBOL_GPL(usb_serial_generic_read_bulk_callback);
+/*
+ * Recover from a halted bulk-in endpoint by clearing the halt condition and
+ * restarting the reads.
+ *
+ * Note that this runs from a dedicated work item rather than from port->work,
+ * which must not be cancelled on close as the line discipline depends on it.
+ */
+void usb_serial_generic_stall_work(struct work_struct *work)
+{
+ struct usb_serial_port *port;
+ struct usb_serial *serial;
+ int ret;
+
+ port = container_of(to_delayed_work(work), struct usb_serial_port,
+ stall_work);
+ serial = port->serial;
+
+ if (!test_and_clear_bit(USB_SERIAL_RX_STALLED, &port->flags))
+ return;
+
+ /*
+ * The sibling URB may still be queued on the halted pipe, or may have
+ * been resubmitted before the stall was noticed, so drop both before
+ * clearing the halt condition.
+ */
+ usb_serial_generic_kill_read_urbs(port);
+
+ ret = usb_clear_halt(serial->dev,
+ usb_rcvbulkpipe(serial->dev,
+ port->bulk_in_endpointAddress));
+ if (ret) {
+ if (ret != -ENODEV && ret != -ESHUTDOWN)
+ dev_err(&port->dev, "failed to clear bulk-in halt: %d\n",
+ ret);
+ return;
+ }
+
+ if (test_bit(USB_SERIAL_THROTTLED, &port->flags))
+ return;
+
+ usb_serial_generic_submit_read_urbs(port, GFP_KERNEL);
+}
+
void usb_serial_generic_write_bulk_callback(struct urb *urb)
{
unsigned long flags;
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 17edc057a311..a9a22b749b60 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1068,6 +1068,8 @@ static int usb_serial_probe(struct usb_interface *interface,
/* Keep this for private driver use for the moment but
should probably go away */
INIT_WORK(&port->work, usb_serial_port_work);
+ INIT_DELAYED_WORK(&port->stall_work,
+ usb_serial_generic_stall_work);
serial->port[i] = port;
port->dev.parent = &interface->dev;
port->dev.driver = NULL;
@@ -1191,6 +1193,7 @@ static void usb_serial_disconnect(struct usb_interface *interface)
usb_serial_port_poison_urbs(port);
wake_up_interruptible(&port->port.delta_msr_wait);
cancel_work_sync(&port->work);
+ cancel_delayed_work_sync(&port->stall_work);
if (device_is_registered(&port->dev))
device_del(&port->dev);
}
@@ -1226,8 +1229,15 @@ int usb_serial_suspend(struct usb_interface *intf, pm_message_t message)
}
}
- for (i = 0; i < serial->num_ports; ++i)
+ /*
+ * The URBs are poisoned first so that no further stall can be reported
+ * before stall recovery, which needs to talk to the device, is
+ * cancelled.
+ */
+ for (i = 0; i < serial->num_ports; ++i) {
usb_serial_port_poison_urbs(serial->port[i]);
+ cancel_delayed_work_sync(&serial->port[i]->stall_work);
+ }
return 0;
}
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 534e6650e2aa..0cd3e2ae772f 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -21,6 +21,7 @@
/* USB serial flags */
#define USB_SERIAL_WRITE_BUSY 0
#define USB_SERIAL_THROTTLED 1
+#define USB_SERIAL_RX_STALLED 2
/**
* usb_serial_port: structure for the specific ports of a device.
@@ -59,6 +60,7 @@
* port.
* @flags: usb serial port flags
* @work: work queue entry for the line discipline waking up.
+ * @stall_work: delayed work entry for bulk-in endpoint stall recovery.
* @dev: pointer to the serial device
*
* This structure is used by the usb-serial core and drivers for the specific
@@ -104,6 +106,7 @@ struct usb_serial_port {
unsigned long flags;
struct work_struct work;
+ struct delayed_work stall_work;
unsigned long sysrq; /* sysrq timeout */
struct device dev;
};
@@ -350,6 +353,7 @@ unsigned int usb_serial_generic_write_room(struct tty_struct *tty);
unsigned int usb_serial_generic_chars_in_buffer(struct tty_struct *tty);
void usb_serial_generic_wait_until_sent(struct tty_struct *tty, long timeout);
void usb_serial_generic_read_bulk_callback(struct urb *urb);
+void usb_serial_generic_stall_work(struct work_struct *work);
void usb_serial_generic_write_bulk_callback(struct urb *urb);
void usb_serial_generic_throttle(struct tty_struct *tty);
void usb_serial_generic_unthrottle(struct tty_struct *tty);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
2026-09-01 3:49 [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint Julian Oes
@ 2026-09-01 4:33 ` Greg Kroah-Hartman
2026-09-01 22:44 ` Julian Oes
2026-09-01 8:28 ` Oliver Neukum
1 sibling, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2026-09-01 4:33 UTC (permalink / raw)
To: Julian Oes; +Cc: Johan Hovold, linux-usb, linux-kernel
On Tue, Sep 01, 2026 at 03:49:49PM +1200, Julian Oes wrote:
> A USB serial port can go permanently silent when its bulk-in endpoint is
> halted: the read URBs complete with -EPIPE, which the generic read
> callback has always treated as fatal, and no further data arrives until
> user space closes and reopens the tty.
Which is good, as the device is obviously broken at that point in time :)
> Recovery from a halted bulk endpoint is CLEAR_FEATURE(ENDPOINT_HALT)
> followed by resubmission of the reads. That needs a control transfer, so
> defer it to a work item as cdc-acm already does.
>
> Use a dedicated work item rather than the existing per-port work.
Ok, this is just llm-generated text. Please rewrite this in your own
words.
> The
> latter is scheduled from every write completion and must not be
> cancelled on close, as the line discipline depends on it. Stall
> recovery resubmits the read URBs and must therefore be cancelled
> wherever the reads are stopped, that is, on close, suspend and
> disconnect.
>
> Schedule the recovery with a short cooldown. The stall can persist for a
> few milliseconds, during which the resubmitted URBs stall again as soon
> as they are queued; without a delay the recovery becomes a busy loop of
> control transfers. Rate-limit the stall message for the same reason, as
> a stall is now a recurring condition rather than a terminal one.
>
> I ran into this with an FTDI FT231X behind a hub that halts the
> adapter's bulk-in endpoint whenever an unrelated CDC ACM device on a
> sibling port is disconnected. With this change the FTDI port keeps
> receiving data across such an event.
This sounds like a broken hub, why not just replace that? It shouldn't
be causing transactions on a different port to stop.
> +/*
> + * Cooldown between attempts to recover a stalled bulk-in endpoint. A stall
> + * caused by a transient fault can persist for a few milliseconds, during which
> + * the resubmitted URBs stall again immediately. Back off a little so that a
> + * persistently halted endpoint is not hammered with control transfers.
> + */
> +#define USB_SERIAL_STALL_COOLDOWN msecs_to_jiffies(10)
control or bulk? This comment does not make much sense.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
2026-09-01 3:49 [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint Julian Oes
2026-09-01 4:33 ` Greg Kroah-Hartman
@ 2026-09-01 8:28 ` Oliver Neukum
2026-09-01 23:13 ` Julian Oes
1 sibling, 1 reply; 8+ messages in thread
From: Oliver Neukum @ 2026-09-01 8:28 UTC (permalink / raw)
To: Julian Oes, Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
On 01.09.26 05:49, Julian Oes wrote:
> A USB serial port can go permanently silent when its bulk-in endpoint is
> halted: the read URBs complete with -EPIPE, which the generic read
> callback has always treated as fatal, and no further data arrives until
> user space closes and reopens the tty.
But why do you get a port stalling?
It seems your hardware is quite broken.
[..]
> Use a dedicated work item rather than the existing per-port work. The
> latter is scheduled from every write completion and must not be
> cancelled on close, as the line discipline depends on it. Stall
> recovery resubmits the read URBs and must therefore be cancelled
> wherever the reads are stopped, that is, on close, suspend and
> disconnect.
Well, I am sorry, but no.
Your conceptual mistake is seeing the recovery from stall
as an indivisible process. It is not, as it has two parts.
Once your port is in a stall, you should send the feature
request to unblock the halt. There is no reason to cancel the
feature request if you close a port. You just need to refrain
from resubmitting the read URB.
In fact, if you were to be really comprehensive you need
to wait for the result of a feature request on the way
when you reopen a port.
> @@ -128,8 +144,16 @@ void usb_serial_generic_close(struct usb_serial_port *port)
> spin_unlock_irqrestore(&port->lock, flags);
> }
> if (port->bulk_in_size) {
> - for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
> - usb_kill_urb(port->read_urbs[i]);
> + usb_serial_generic_kill_read_urbs(port);
> + /*
> + * The read URBs are dead now so no further stall can be
> + * reported, but stall recovery may already be running and may
> + * have resubmitted them. Wait for it to finish before killing
> + * the URBs for good.
> + */
> + cancel_delayed_work_sync(&port->stall_work);
> + usb_serial_generic_kill_read_urbs(port);
And that is a race condition. Rekilling does not help reliably.
If your timing is unlucky enough any subsequent operation can be a nop.
A correct sequence would be something like
poison URBs -> cancel the works -> unpoison the URBs
Regards
Oliver
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
2026-09-01 4:33 ` Greg Kroah-Hartman
@ 2026-09-01 22:44 ` Julian Oes
0 siblings, 0 replies; 8+ messages in thread
From: Julian Oes @ 2026-09-01 22:44 UTC (permalink / raw)
To: gregkh; +Cc: johan, oneukum, linux-kernel, linux-usb, Julian Oes
On Tue, Sep 01, 2026 at 06:33:15AM +0200, Greg Kroah-Hartman wrote:
> This sounds like a broken hub, why not just replace that? It shouldn't
> be causing transactions on a different port to stop.
I agree that ideally my hub would not do that. However, my assumption is
that it's not my specific hub that is broken, but that this hub (or
other hubs on the same chip) have that sort of issue, and I was hoping
to be able to fix it for me and others in software, rather than brush it
off as a hardware issue.
From what I understand an endpoint halt is spec-defined as
host-recoverable. In the current state (without the patch) the read
never returns data again and the user doesn't know that it needs
restarting.
My patch is meant to do the same pattern that cdc-acm already does:
EVENT_RX_STALL → kill → usb_clear_halt → resubmit.
> > +#define USB_SERIAL_STALL_COOLDOWN msecs_to_jiffies(10)
>
> control or bulk? This comment does not make much sense.
Sorry, it's not clear. Both: the halted endpoint is the bulk-in one,
but the CLEAR_FEATURE that clears it goes to the control pipe, so one
recovery attempt is a control transfer plus a bulk resubmit.
Without the delay I saw six stalls in six milliseconds. The clear-halt
succeeds every time, but the hub re-stalls the endpoint as soon as a
read is queued, until the transient is over.
The delay is there to stop that becoming a tight loop.
I can reword it for v3, depending on whether you agree that a stall
needs clearing at all.
For a bit of context: in the past 14 years, I have been working with
various FTDI and similar USB devices, and it's possible that I have seen
this issue every so often. Usually, I would just re-open screen, try a
different USB port or hub, restart the hub, etc. It's only this time
that I stumbled on a reproducible case, and had the capacity to dig
deeper using an LLM.
I have been using the v1 patch that I submitted in May ever since and
have not noticed such issues anymore.
Thanks,
Julian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
2026-09-01 8:28 ` Oliver Neukum
@ 2026-09-01 23:13 ` Julian Oes
2026-09-02 4:42 ` Michal Pecio
0 siblings, 1 reply; 8+ messages in thread
From: Julian Oes @ 2026-09-01 23:13 UTC (permalink / raw)
To: oneukum; +Cc: gregkh, johan, linux-kernel, linux-usb, Julian Oes
On Tue, Sep 01, 2026 at 10:28:08AM +0200, Oliver Neukum wrote:
> But why do you get a port stalling?
> It seems your hardware is quite broken.
Maybe, yes, but as I wrote to Greg, I believe I have seen this (or
similar stalls) over the years in the past with various hardware.
Maybe it's just me but if it is not, it would be nice to fix it for
others too.
I am also trying to put together a reproducer with dummy_hcd and
raw-gadget that halts the bulk-in endpoint on demand, so this does not
depend on my hub. I will report back once I have run it.
> Your conceptual mistake is seeing the recovery from stall
> as an indivisible process. It is not, as it has two parts.
> Once your port is in a stall, you should send the feature
> request to unblock the halt. There is no reason to cancel the
> feature request if you close a port. You just need to refrain
> from resubmitting the read URB.
That makes sense. I'll try to fix that for v3.
> In fact, if you were to be really comprehensive you need
> to wait for the result of a feature request on the way
> when you reopen a port.
With the sequence below in close(), nothing is in flight by the time
close() returns, so as I understand it a reopen has nothing left to wait
for. Does that work?
> And that is a race condition. Rekilling does not help reliably.
> If your timing is unlucky enough any subsequent operation can be a nop.
> A correct sequence would be something like
>
> poison URBs -> cancel the works -> unpoison the URBs
Ok, what about:
if (port->bulk_in_size) {
for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
usb_poison_urb(port->read_urbs[i]);
cancel_delayed_work_sync(&port->stall_work);
for (i = 0; i < ARRAY_SIZE(port->read_urbs); ++i)
usb_unpoison_urb(port->read_urbs[i]);
}
I will send it as v3 once it is clearer whether this is worth doing at
all.
Thanks,
Julian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
2026-09-01 23:13 ` Julian Oes
@ 2026-09-02 4:42 ` Michal Pecio
2026-09-02 8:40 ` Oliver Neukum
2026-09-02 14:33 ` Alan Stern
0 siblings, 2 replies; 8+ messages in thread
From: Michal Pecio @ 2026-09-02 4:42 UTC (permalink / raw)
To: Julian Oes; +Cc: oneukum, gregkh, johan, linux-kernel, linux-usb
On Wed, 2 Sep 2026 11:13:53 +1200, Julian Oes wrote:
> On Tue, Sep 01, 2026 at 10:28:08AM +0200, Oliver Neukum wrote:
> > But why do you get a port stalling?
> > It seems your hardware is quite broken.
>
> Maybe, yes, but as I wrote to Greg, I believe I have seen this (or
> similar stalls) over the years in the past with various hardware.
> Maybe it's just me but if it is not, it would be nice to fix it for
> others too.
What you are probably seeing is USB 2.0 hub(s) returning STALL
handshake when a transaction attempt with downstream low/full-speed
device fails three times. See USB 2.0 section 11.17.1 page 364.
If that's the case, the device endpoint isn't actually halted and you
would see the traffic resume if you simply ignored the error and kept
resubmitting until communication is restored.
That being said, calling usb_clear_halt() is indeed the only recovery
supported by USB specs, both for -EPIPE and -EPROTO or similar. Linux
has traditionally ignored this and things are quite broken sometimes,
particularly with xhci-hcd, even if you call usb_clear_halt().
I suppose you can try this and see how it works, and if you run into
xhci-hcd bugs we could try to fix them too. The -EPIPE case is easier
because drivers don't rely on out of spec behavior of the USB stack.
Related discussion: (gonna need *a lot* of popcorn for that one)
https://lore.kernel.org/linux-usb/261996a8-7ad4-4df2-a469-f6602da71255@suse.com/
Alan Stern tried to come up with some solution in USB core, but not sure
how far that got.
It seems there is only one risk of usb_clear_halt() in such cases:
- you send a packet to an OUT endpoint
- and the device accepts it but you never receive the ACK
- even after re-sending three times
- you call usb_clear_halt() and queue the same packet again
- the device may accept the packet twice
The recommended solution is to query the device by class-defined means
to determine whether it has received the apparently lost packet or not,
or to put it into some known and desired state. Common problem: nobody
knows how to do that with given device.
(And the same could happen in IN direction if you usb_clear_halt() after
a successful transfer, but drivers are unlikely to do that).
> > Your conceptual mistake is seeing the recovery from stall
> > as an indivisible process. It is not, as it has two parts.
> > Once your port is in a stall, you should send the feature
> > request to unblock the halt. There is no reason to cancel the
> > feature request if you close a port. You just need to refrain
> > from resubmitting the read URB.
>
> That makes sense. I'll try to fix that for v3.
Yes, if you are going there, just clear the halt and reset everything
unconditionally. Then the pipe will be ready for new transfers.
One note about rate limiting: it would perhaps make sense to perform
the first attempt ASAP and only slow down for retries. But arguably
anything at all is better than just giving up like now.
Regards,
Michal
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
2026-09-02 4:42 ` Michal Pecio
@ 2026-09-02 8:40 ` Oliver Neukum
2026-09-02 14:33 ` Alan Stern
1 sibling, 0 replies; 8+ messages in thread
From: Oliver Neukum @ 2026-09-02 8:40 UTC (permalink / raw)
To: Michal Pecio, Julian Oes; +Cc: oneukum, gregkh, johan, linux-kernel, linux-usb
On 02.09.26 06:42, Michal Pecio wrote:
> On Wed, 2 Sep 2026 11:13:53 +1200, Julian Oes wrote:
>> On Tue, Sep 01, 2026 at 10:28:08AM +0200, Oliver Neukum wrote:
> What you are probably seeing is USB 2.0 hub(s) returning STALL
> handshake when a transaction attempt with downstream low/full-speed
> device fails three times. See USB 2.0 section 11.17.1 page 364.
Nasty.
> If that's the case, the device endpoint isn't actually halted and you
> would see the traffic resume if you simply ignored the error and kept
> resubmitting until communication is restored.
>
> That being said, calling usb_clear_halt() is indeed the only recovery
> supported by USB specs, both for -EPIPE and -EPROTO or similar. Linux
> has traditionally ignored this and things are quite broken sometimes,
> particularly with xhci-hcd, even if you call usb_clear_halt().
I think our record is better with -EPIPE.
The question is what we have to lose. Frankly, compared to the
status quo, nothing.
[..]
I don't have popcorn, but I do have cooled, sugar-free beverages ready.
> It seems there is only one risk of usb_clear_halt() in such cases:
> - you send a packet to an OUT endpoint
> - and the device accepts it but you never receive the ACK
> - even after re-sending three times
> - you call usb_clear_halt() and queue the same packet again
> - the device may accept the packet twice
I am afraid this is the time to be pedantic, because I don't
see the connection to usb_clear_halt().
The fundamental disagreement is on whether a packet has arrived
or not, isn't it? So the fundamental issue is whether IO should
be retried, not whether you clear a halt in between.
If you guess wrong you either transmit data twice or not at all.
And there is no generic mechanism to remedy that. Do you have
a proposal how one would look like?
However, eventually new data will need to be transmitted
or the device queried for newly received data. If that is to work,
we'll need to, well, do IO.
Our choices are whether
a) we retry IO before we do so
b) whether we try to clear a halt before that
Technically these decisions are independent of one another.
However, the spec says that we should clear a halt.
So I need to ask: Is there a situation in which we would
make matters worse by clearing the halt?
> One note about rate limiting: it would perhaps make sense to perform
> the first attempt ASAP and only slow down for retries. But arguably
> anything at all is better than just giving up like now.
And again, you make me ask whether a helper for that should
go into usbcore.
Regards
Oliver
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
2026-09-02 4:42 ` Michal Pecio
2026-09-02 8:40 ` Oliver Neukum
@ 2026-09-02 14:33 ` Alan Stern
1 sibling, 0 replies; 8+ messages in thread
From: Alan Stern @ 2026-09-02 14:33 UTC (permalink / raw)
To: Michal Pecio; +Cc: Julian Oes, oneukum, gregkh, johan, linux-kernel, linux-usb
On Wed, Sep 02, 2026 at 06:42:04AM +0200, Michal Pecio wrote:
> What you are probably seeing is USB 2.0 hub(s) returning STALL
> handshake when a transaction attempt with downstream low/full-speed
> device fails three times. See USB 2.0 section 11.17.1 page 364.
>
> If that's the case, the device endpoint isn't actually halted and you
> would see the traffic resume if you simply ignored the error and kept
> resubmitting until communication is restored.
>
> That being said, calling usb_clear_halt() is indeed the only recovery
> supported by USB specs, both for -EPIPE and -EPROTO or similar. Linux
> has traditionally ignored this and things are quite broken sometimes,
> particularly with xhci-hcd, even if you call usb_clear_halt().
>
> I suppose you can try this and see how it works, and if you run into
> xhci-hcd bugs we could try to fix them too. The -EPIPE case is easier
> because drivers don't rely on out of spec behavior of the USB stack.
>
> Related discussion: (gonna need *a lot* of popcorn for that one)
> https://lore.kernel.org/linux-usb/261996a8-7ad4-4df2-a469-f6602da71255@suse.com/
>
> Alan Stern tried to come up with some solution in USB core, but not sure
> how far that got.
For what it's worth, I did get started on that project and made some
good progress. But it started becoming rather complicated, particularly
when taking into account that the core would need to stop trying to send
Clear-Halt requests when it's about to do a Set-Interface or
Set-Configuration, or handle a disconnection. Then I got distracted
with other things and never finished that part of the patch.
Maybe I can find time to go back and work on it some more. It's likely
to take a while...
Alan Stern
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-02 14:33 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01 3:49 [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint Julian Oes
2026-09-01 4:33 ` Greg Kroah-Hartman
2026-09-01 22:44 ` Julian Oes
2026-09-01 8:28 ` Oliver Neukum
2026-09-01 23:13 ` Julian Oes
2026-09-02 4:42 ` Michal Pecio
2026-09-02 8:40 ` Oliver Neukum
2026-09-02 14:33 ` Alan Stern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox