All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 0/2] xhci: Sort out the TD skipping business
Date: Wed, 7 Oct 2026 09:49:52 +0200	[thread overview]
Message-ID: <20261007094952.28bfa51e.michal.pecio@gmail.com> (raw)

Hi Mathias,

I see that you have picked this series for 7.4:
https://lore.kernel.org/linux-usb/20260804120110.01bda0e2.michal.pecio@gmail.com/

There is unfortunately a small bug in "Shorten the TD skipping loop":
old_trb_comp_code is assigned before being used by the !trb_in_td()
block, which means that we use the *current* trb_comp_code instead.

 	ep_ring->old_trb_comp_code = trb_comp_code;

 	if (ring_xrun_event)
 		return 0;
 
+	/* Handle events not referencing the current TD */
+	if (!trb_in_td(td, ep_trb_dma)) {
[...]
+		if (xhci_spurious_success_tx_event(xhci, ep_ring))
			// ^-- uses old_trb_comp_code

This causes false warnings when running UVC with non-power-of-2 alt
setting on HCs like VIA, which generate Success after a mid-TD Short
Packet. So this will surely bother users with such workloads.

It happened because I tried to resuse existing ring_xrun_event check
and remove an identical check from the !trb_in_td() block, so I placed
the block after this check.

Revised patch places it before the old_trb_comp_code assignment and
retains a duplicate ring_xrun_event check in the moved code, I'm not
trying to be clever anymore. Patch 2/2 is a necessary rebase.

It is also a pre-existing bug that in some cases the function bails out
without updating old_trb_comp_code, even though we know that the prior
TD cannot generate more events (e.g. we are handling ring underrun).
But that's a separate issue that causes missing warnings in rare cases
rather than false warnings in common cases.

Regards,
Michal

             reply	other threads:[~2026-10-07  7:49 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07  7:49 Michal Pecio [this message]
2026-10-07  7:51 ` [PATCH v2 1/2] usb: xhci: Shorten the TD skipping loop Michal Pecio
2026-10-07  7:52 ` [PATCH 2/2] usb: xhci: Rework and improve the TD matching and skipping logic Michal Pecio
2026-10-07  9:38 ` [PATCH v2 0/2] xhci: Sort out the TD skipping business Mathias Nyman

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=20261007094952.28bfa51e.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.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.