From: Mathias Nyman <mathias.nyman@linux.intel.com>
To: Michal Pecio <michal.pecio@gmail.com>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/1] usb: xhci: Queue URB_ZERO_PACKET as one TD
Date: Tue, 9 Sep 2025 16:04:33 +0300 [thread overview]
Message-ID: <6ca18b05-80d4-4988-bb08-3cad003e10f4@linux.intel.com> (raw)
In-Reply-To: <20250908130128.7ed81912.michal.pecio@gmail.com>
On 8.9.2025 14.01, Michal Pecio wrote:
> Hi Mathias,
>
> I wanted to finish and send v2 of "Simplify TD cancellation and drop
> some states" over the weekend, but I encountered an annoying roadblock
> and I need your input.
>
> Basically, there is a bug: URB_ZERO_PACKET is queued as two TDs, and
> when the first TD halts, the driver simply advances to the second one.
>
> I found that extending the event handler to cover this case requires
> multiple changes:
>
> 1. obviously, all TDs must be cancelled, not just the current one
> 2. they may be given back in weird order (waiting for Set Deq), so
> we need to store the status on urb_priv and override td->status
> 3. xhci_invalidate_cancelled_tds() would need to recognize multiple
> halted TDs on the same URB as valid
>
> This is doable, and I have already implemented most of it in that
> series, but there is an alternative solution: simply stop worrying
> about halted multi-TD URBs, because this is the only case and it
> can be converted to pretend that it's just one TD per URB as usual.
>
> If you are OK with this patch, cancellation logic will be simpler,
> because this time there really are no remaining cases of multi-TD
> URBs except isochronous. This is clear in xhci_urb_enqueue():
>
Adding the zero-length TRB to the original TD when we need to send a
zero-length packet would simplify things, and I would otherwise fully
support this, but the xHCI spec is pretty clear that it requires a
dedicated TD for zero-length transactions.
See xhci spec section 4.9.1:
"To generate a “zero-length” USB transaction, software shall explicitly
define a TD with a single Transfer TRB, and its TRB Transfer Length
field shall equal ‘0’. Note that this TD may include non-Transfer TRBs,
e.g. an Event Data or Link TRB."
Thanks
Mathias
next prev parent reply other threads:[~2025-09-09 13:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-08 11:01 [PATCH 0/1] usb: xhci: Queue URB_ZERO_PACKET as one TD Michal Pecio
2025-09-08 11:02 ` [PATCH 1/1] " Michal Pecio
2025-09-09 13:04 ` Mathias Nyman [this message]
2025-09-09 17:38 ` [PATCH 0/1] " Michal Pecio
2025-09-09 22:57 ` Mathias Nyman
2025-09-10 0:03 ` Michal Pecio
2025-09-10 0:15 ` Michal Pecio
2025-09-10 21:37 ` Michal Pecio
2025-09-22 8:16 ` Michal Pecio
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=6ca18b05-80d4-4988-bb08-3cad003e10f4@linux.intel.com \
--to=mathias.nyman@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=michal.pecio@gmail.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.