From: Julian Oes <julian@oes.ch>
To: oneukum@suse.com
Cc: gregkh@linuxfoundation.org, johan@kernel.org,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
Julian Oes <julian@oes.ch>
Subject: Re: [PATCH v2] USB: serial: generic: recover from a stalled bulk-in endpoint
Date: Wed, 2 Sep 2026 11:13:53 +1200 [thread overview]
Message-ID: <20260901231355.114733-1-julian@oes.ch> (raw)
In-Reply-To: <746c4df4-abd5-4e04-9edc-3ff8f17506bf@suse.com>
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
next prev parent reply other threads:[~2026-09-01 23:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2026-09-02 4:42 ` Michal Pecio
2026-09-02 8:40 ` Oliver Neukum
2026-09-02 14:33 ` Alan Stern
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=20260901231355.114733-1-julian@oes.ch \
--to=julian@oes.ch \
--cc=gregkh@linuxfoundation.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=oneukum@suse.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox